Skip to content

Commit

Permalink
Do not generate invalid setter methods for attributes that are either…
Browse files Browse the repository at this point in the history
… constant or not assignable (ie, operator= is deleted or not public).
  • Loading branch information
cfis committed Nov 17, 2024
1 parent 67f34c1 commit 1193d43
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 20 deletions.
11 changes: 8 additions & 3 deletions rice/Data_Type.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,14 @@ namespace Rice
if (access == AttrAccess::ReadWrite || access == AttrAccess::Read)
detail::NativeAttributeGet<Attribute_T>::define(klass_, name, std::forward<Attribute_T>(attribute));

// Define native attribute setter
if (access == AttrAccess::ReadWrite || access == AttrAccess::Write)
detail::NativeAttributeSet<Attribute_T>::define(klass_, name, std::forward<Attribute_T>(attribute));
using Attr_T = typename detail::NativeAttributeSet<Attribute_T>::Attr_T;
if constexpr (!std::is_const_v<Attr_T> &&
(std::is_fundamental_v<Attr_T> || std::is_assignable_v<Attr_T, Attr_T>))
{
// Define native attribute setter
if (access == AttrAccess::ReadWrite || access == AttrAccess::Write)
detail::NativeAttributeSet<Attribute_T>::define(klass_, name, std::forward<Attribute_T>(attribute));
}

return *this;
}
Expand Down
7 changes: 3 additions & 4 deletions rice/detail/NativeAttributeSet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@ namespace Rice
{
public:
using NativeAttribute_T = NativeAttributeSet<Attribute_T>;

using T = typename attribute_traits<Attribute_T>::attr_type;
using T_Unqualified = remove_cv_recursive_t<T>;
using Attr_T = typename attribute_traits<Attribute_T>::attr_type;
using T_Unqualified = remove_cv_recursive_t<Attr_T>;
using Receiver_T = typename attribute_traits<Attribute_T>::class_type;

public:
// Register attribute getter/setter with Ruby
static void define(VALUE klass, std::string name, Attribute_T attribute);
Expand Down
28 changes: 16 additions & 12 deletions rice/detail/NativeAttributeSet.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ namespace Rice::detail
NativeAttribute_T* nativeAttribute = new NativeAttribute_T(klass, name, std::forward<Attribute_T>(attribute));
std::unique_ptr<Native> native(nativeAttribute);

if (std::is_const_v<std::remove_pointer_t<T>>)
{
throw std::runtime_error(name + " is readonly");
}

// Define the write method name
std::string setter = name + "=";

Expand Down Expand Up @@ -51,31 +46,40 @@ namespace Rice::detail
template<typename Attribute_T>
inline VALUE NativeAttributeSet<Attribute_T>::operator()(int argc, VALUE* argv, VALUE self)
{
if constexpr (std::is_fundamental_v<intrinsic_type<T>> && std::is_pointer_v<T>)
if constexpr (std::is_fundamental_v<intrinsic_type<Attr_T>> && std::is_pointer_v<Attr_T>)
{
static_assert(true, "An fundamental value, such as an integer, cannot be assigned to an attribute that is a pointer");
static_assert(true, "An fundamental value, such as an integer, cannot be assigned to an attribute that is a pointer.");
}
else if constexpr (std::is_same_v<intrinsic_type<T>, std::string> && std::is_pointer_v<T>)
else if constexpr (std::is_same_v<intrinsic_type<Attr_T>, std::string> && std::is_pointer_v<Attr_T>)
{
static_assert(true, "An string cannot be assigned to an attribute that is a pointer");
static_assert(true, "An string cannot be assigned to an attribute that is a pointer.");
}

if (argc != 1)
{
throw std::runtime_error("Incorrect number of parameter set to attribute writer");
throw std::runtime_error("Incorrect number of parameters for setting attribute. Attribute: " + this->name_);
}

VALUE value = argv[0];

if constexpr (!std::is_null_pointer_v<Receiver_T>)
if constexpr (!std::is_null_pointer_v<Receiver_T> &&
!std::is_const_v<Attr_T> &&
(std::is_fundamental_v<Attr_T> || std::is_assignable_v<Attr_T, Attr_T>))
{
Receiver_T* nativeSelf = From_Ruby<Receiver_T*>().convert(self);
nativeSelf->*attribute_ = From_Ruby<T_Unqualified>().convert(value);
}
else if constexpr (!std::is_const_v<std::remove_pointer_t<T>>)
else if constexpr (std::is_null_pointer_v<Receiver_T> &&
!std::is_const_v<Attr_T> &&
(std::is_fundamental_v<Attr_T> || std::is_assignable_v<Attr_T, Attr_T>))
{
*attribute_ = From_Ruby<T_Unqualified>().convert(value);
}
else
{
// Should never get here because define_attr won't compile this code, but just in case!
throw std::invalid_argument("Could not set attribute. Attribute: " + this->name_);
}

return value;
}
Expand Down
45 changes: 44 additions & 1 deletion test/test_Attribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ namespace
{
};

class NotAssignable
{
NotAssignable& operator=(const NotAssignable&) = delete;
};

struct DataStruct
{
static inline float staticFloat = 1.0;
Expand All @@ -29,7 +34,9 @@ namespace
std::string readWriteString = "Read Write";
int writeInt = 0;
const char* readChars = "Read some chars!";
const int constInt = 5;
SomeClass someClass;
NotAssignable notAssignable;

std::string inspect()
{
Expand Down Expand Up @@ -82,6 +89,42 @@ TESTCASE(attributes)
ASSERT_EQUAL("Set a string", detail::From_Ruby<std::string>().convert(result.value()));
}

TESTCASE(const_attribute)
{
Class c = define_class<DataStruct>("DataStruct")
.define_constructor(Constructor<DataStruct>())
.define_attr("const_int", &DataStruct::constInt);

Data_Object<DataStruct> o = c.call("new");
const DataStruct* dataStruct = o.get();

ASSERT_EXCEPTION_CHECK(
Exception,
o.call("const_int=", 5),
ASSERT(std::string(ex.what()).find("undefined method `const_int='") == 0)
);
}

TESTCASE(not_copyable_attribute)
{
Class notAssignableClass = define_class<NotAssignable>("NotAssignable")
.define_constructor(Constructor<NotAssignable>());

Class c = define_class<DataStruct>("DataStruct")
.define_constructor(Constructor<DataStruct>())
.define_attr("not_assignable", &DataStruct::notAssignable);

Data_Object<NotAssignable> notAssignable = notAssignableClass.call("new");

Data_Object<DataStruct> o = c.call("new");

ASSERT_EXCEPTION_CHECK(
Exception,
o.call("not_assignable=", notAssignable),
ASSERT(std::string(ex.what()).find("undefined method `not_assignable='") == 0)
);
}

TESTCASE(static_attributes)
{
Class c = define_class<DataStruct>("DataStruct")
Expand Down Expand Up @@ -144,4 +187,4 @@ TESTCASE(not_defined)
c.define_attr("some_class", &DataStruct::someClass),
ASSERT_EQUAL(message, ex.what())
);
}
}

0 comments on commit 1193d43

Please sign in to comment.