Skip to content
This repository was archived by the owner on Mar 26, 2020. It is now read-only.

Countable Enums #349

Open
steipete wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Countable Enums #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53bc5ad
Add template specialization for numeric limits to allow enum size query
steipete Jan 11, 2018
53e115a
Update test suite to add numeric_limits specializations
steipete Jan 11, 2018
fc8706b
Add enum increment operator
steipete Jan 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/source/CppGenerator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ class CppGenerator(spec: Spec) extends Generator(spec) {
w.w(s"constexpr $self operator~($self x) noexcept").braced {
w.wl(s"return static_cast<$self>(~static_cast<$flagsType>(x));")
}
} else {
// Define useful operators for enum
Copy link
Contributor

@artwyman artwyman Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment in the example generated code. I wonder why it would make sense to define only one such operator, rather than a full set.

Increment in particular has the issue that it can cause overflow/underflow and result in illegal values.

w.w(s"constexpr $self operator++($self const& r, int increment) noexcept").braced {
w.wl(s"return $self(($underlyingType)r + increment);")
}
}
},
w => {
Expand All @@ -106,6 +111,13 @@ class CppGenerator(spec: Spec) extends Generator(spec) {
w.wl(s"return std::hash<$underlyingType>()(static_cast<$underlyingType>(type));")
}
}
w.wl("template <>")
w.w(s"class numeric_limits<$fqSelf> : public numeric_limits<$underlyingType>").bracedSemi {
w.wl("public:")
w.wl("static constexpr bool is_specialized = true;")
if(!e.flags) w.wl(s"static constexpr $fqSelf min() noexcept { return $fqSelf::${idCpp.enum(normalEnumOptions(e).head.ident.name)}; }")
w.wl(s"static constexpr $fqSelf max() noexcept { return " + (if(e.flags) s"static_cast<$fqSelf>(${normalEnumOptions(e).size})" else s"$fqSelf::${idCpp.enum(normalEnumOptions(e).last.ident.name)}") +"; }")
}
}
)
}
Expand Down
6 changes: 6 additions & 0 deletions test-suite/generated-src/cpp/access_flags.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,11 @@ struct hash<::testsuite::access_flags> {
return std::hash<unsigned>()(static_cast<unsigned>(type));
}
};
template <>
class numeric_limits<::testsuite::access_flags> : public numeric_limits<unsigned> {
public:
Copy link
Contributor

@artwyman artwyman Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be outdented

static constexpr bool is_specialized = true;
static constexpr ::testsuite::access_flags max() noexcept { return static_cast<::testsuite::access_flags>(9); }
Copy link
Contributor

@artwyman artwyman Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this calculation is correct for flags. A flag type like this may have 9 values, but its range is a full 9 bits.

};

} // namespace std
10 changes: 10 additions & 0 deletions test-suite/generated-src/cpp/color.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ enum class color : int {
INDIGO,
VIOLET,
};
constexpr color operator++(color const& r, int increment) noexcept {
Copy link
Contributor

@artwyman artwyman Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you adding increment, but not decrement or any other arithmatic operations? Feels like a partial feature.

Copy link

@thisismyworkgithub thisismyworkgithub Jan 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the increment operator is being used as a way to iterate over a set of enum values, it may be more C++ idiomatic to define an enum_iterator class of some kind that follows a general set of rules to reach all values. Otherwise, operator++ would be sequencing through all combinations of valid flags when used on bit-flag enums, or possibly leading to illegal values (as previously mentioned by @artwyman)

return color((int)r + increment);
}

} // namespace testsuite

Expand All @@ -32,5 +35,12 @@ struct hash<::testsuite::color> {
return std::hash<int>()(static_cast<int>(type));
}
};
template <>
class numeric_limits<::testsuite::color> : public numeric_limits<int> {
public:
static constexpr bool is_specialized = true;
static constexpr ::testsuite::color min() noexcept { return ::testsuite::color::RED; }
static constexpr ::testsuite::color max() noexcept { return ::testsuite::color::VIOLET; }
};

} // namespace std
6 changes: 6 additions & 0 deletions test-suite/generated-src/cpp/empty_flags.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,11 @@ struct hash<::testsuite::empty_flags> {
return std::hash<unsigned>()(static_cast<unsigned>(type));
}
};
template <>
class numeric_limits<::testsuite::empty_flags> : public numeric_limits<unsigned> {
public:
static constexpr bool is_specialized = true;
static constexpr ::testsuite::empty_flags max() noexcept { return static_cast<::testsuite::empty_flags>(0); }
};

} // namespace std