Skip to content

Idea: Adding from_string to enum metafunction #1185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Aug 4, 2024
Merged

Idea: Adding from_string to enum metafunction #1185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
37535c0
Added from_string to enum metafunction
tsoj Jul 28, 2024
9a25f87
Changed string to string_view
tsoj Jul 28, 2024
8587a6d
Changed string to string_view in test result now too
tsoj Jul 30, 2024
e426805
Enable from_string for flag_enums too, and use contract violation han…
hsutter Jul 30, 2024
918584b
Fix typo in regression test case
hsutter Jul 31, 2024
7c02229
For flag_enums, let from_string take a "( list, of_multiple, flags )"
hsutter Jul 31, 2024
654fff0
Changed flag_enum to_string from (a,b) to (a|b)
hsutter Aug 1, 2024
9a73208
Provide {to|from}_{string|code}
hsutter Aug 2, 2024
ab43b7d
Avoid repeated eval of to_string(prefix)
hsutter Aug 3, 2024
0a4bd0d
Merge branch 'main' into enum_from_string
hsutter Aug 4, 2024
d3fb6bf
Merge branch 'main' into enum_from_string
hsutter Aug 4, 2024
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
55 changes: 45 additions & 10 deletions include/cpp2regex.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,11 @@ public: constexpr auto operator=(expression_flags const& that) -> expression_fla
public: constexpr expression_flags(expression_flags&& that) noexcept;
public: constexpr auto operator=(expression_flags&& that) noexcept -> expression_flags& ;
public: [[nodiscard]] auto operator<=>(expression_flags const& that) const& -> std::strong_ordering = default;
public: [[nodiscard]] auto to_string_impl(cpp2::impl::in<std::string_view> prefix, cpp2::impl::in<std::string_view> separator) const& -> std::string;
public: [[nodiscard]] auto to_string() const& -> std::string;
public: [[nodiscard]] auto to_code() const& -> std::string;
public: [[nodiscard]] static auto from_string(cpp2::impl::in<std::string_view> s) -> expression_flags;
public: [[nodiscard]] static auto from_code(cpp2::impl::in<std::string_view> s) -> expression_flags;

#line 55 "cpp2regex.h2"
};
Expand Down Expand Up @@ -1540,21 +1544,51 @@ constexpr expression_flags::expression_flags(expression_flags&& that) noexcept
constexpr auto expression_flags::operator=(expression_flags&& that) noexcept -> expression_flags& {
_value = std::move(that)._value;
return *this;}
[[nodiscard]] auto expression_flags::to_string() const& -> std::string{
[[nodiscard]] auto expression_flags::to_string_impl(cpp2::impl::in<std::string_view> prefix, cpp2::impl::in<std::string_view> separator) const& -> std::string{

std::string _ret {"("};
std::string ret {"("};

std::string _comma {};
std::string sep {};
if ((*this) == none) {return "(none)"; }
if (((*this) & case_insensitive) == case_insensitive) {_ret += _comma + "case_insensitive";_comma = ", ";}
if (((*this) & multiple_lines) == multiple_lines) {_ret += _comma + "multiple_lines";_comma = ", ";}
if (((*this) & single_line) == single_line) {_ret += _comma + "single_line";_comma = ", ";}
if (((*this) & no_group_captures) == no_group_captures) {_ret += _comma + "no_group_captures";_comma = ", ";}
if (((*this) & perl_code_syntax) == perl_code_syntax) {_ret += _comma + "perl_code_syntax";_comma = ", ";}
if (((*this) & perl_code_syntax_in_classes) == perl_code_syntax_in_classes) {_ret += _comma + "perl_code_syntax_in_classes";_comma = ", ";}
return cpp2::move(_ret) + ")";
if (((*this) & case_insensitive) == case_insensitive) {ret += sep + cpp2::to_string(prefix) + "case_insensitive";sep = separator;}
if (((*this) & multiple_lines) == multiple_lines) {ret += sep + cpp2::to_string(prefix) + "multiple_lines";sep = separator;}
if (((*this) & single_line) == single_line) {ret += sep + cpp2::to_string(prefix) + "single_line";sep = separator;}
if (((*this) & no_group_captures) == no_group_captures) {ret += sep + cpp2::to_string(prefix) + "no_group_captures";sep = separator;}
if (((*this) & perl_code_syntax) == perl_code_syntax) {ret += sep + cpp2::to_string(prefix) + "perl_code_syntax";sep = separator;}
if (((*this) & perl_code_syntax_in_classes) == perl_code_syntax_in_classes) {ret += sep + cpp2::to_string(prefix) + "perl_code_syntax_in_classes";sep = separator;}
return cpp2::move(ret) + ")";
}

[[nodiscard]] auto expression_flags::to_string() const& -> std::string { return to_string_impl("", ", "); }
[[nodiscard]] auto expression_flags::to_code() const& -> std::string { return to_string_impl("expression_flags::", " | "); }
[[nodiscard]] auto expression_flags::from_string(cpp2::impl::in<std::string_view> s) -> expression_flags{
Copy link
Contributor

Choose a reason for hiding this comment

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

@hsutter Why do all of these lines start at column 0? There is leading whitespace in the metafunction. Is it because the whitespace is tabs, and those are being stripped somehow?

Copy link
Owner

@hsutter hsutter Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Good question, I don't know offhand.


auto ret {none};
do {{
for ( auto const& x : cpp2::string_util::split_string_list(s) ) {
if ("case_insensitive" == x) {ret |= case_insensitive;}
else {if ("multiple_lines" == x) {ret |= multiple_lines;}
else {if ("single_line" == x) {ret |= single_line;}
else {if ("no_group_captures" == x) {ret |= no_group_captures;}
else {if ("perl_code_syntax" == x) {ret |= perl_code_syntax;}
else {if ("perl_code_syntax_in_classes" == x) {ret |= perl_code_syntax_in_classes;}
else {if ("none" == x) {ret |= none;}
else {goto BREAK_outer;}
#line 1 "cpp2regex.h2"
}}}}}}
}

return ret;
} CPP2_CONTINUE_BREAK(outer) }
while (
false
);
CPP2_UFCS(report_violation)(cpp2::type_safety, CPP2_UFCS(c_str)(("can't convert string '" + cpp2::to_string(s) + "' to flag_enum of type expression_flags")));
return none;
}

[[nodiscard]] auto expression_flags::from_code(cpp2::impl::in<std::string_view> s) -> expression_flags{
std::string str {s}; return from_string(cpp2::string_util::replace_all(cpp2::move(str), "expression_flags::", "")); }
template <typename Iter> match_group<Iter>::match_group(auto const& start_, auto const& end_, auto const& matched_)
: start{ start_ }
, end{ end_ }
Expand All @@ -1564,6 +1598,7 @@ template <typename Iter> match_return<Iter>::match_return(auto const& matched_,
: matched{ matched_ }
, pos{ pos_ }{}
template <typename Iter> match_return<Iter>::match_return(){}

#line 38 "cpp2regex.h2"
//-----------------------------------------------------------------------
//
Expand Down
34 changes: 34 additions & 0 deletions include/cpp2util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,40 @@ using _uchar = unsigned char; // normally use u8 instead

namespace string_util {

// Break a string_view into a vector of views of simple qidentifier
// substrings separated by other characters
auto split_string_list(std::string_view str)
-> std::vector<std::string_view>
{
std::vector<std::string_view> ret;

auto is_id_char = [](char c) {
return std::isalnum(c) || c == '_';
};

auto pos = 0;
while( pos < std::ssize(str) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick, if you care: style difference between this while and the next three while/if, spaces inside parens here and not below, space after keyword below and not here.

// Skip non-alnum
while (pos < std::ssize(str) && !is_id_char(str[pos])) {
++pos;
}
auto start = pos;

// Find the end of the current component
while (pos < std::ssize(str) && is_id_char(str[pos])) {
++pos;
}

// Add nonempty substring to the vector
if (start < pos) {
ret.emplace_back(str.substr(start, pos - start));
}
}

return ret;
}


// From https://stackoverflow.com/questions/216823/how-to-trim-a-stdstring

// Trim from start (in place)
Expand Down
15 changes: 15 additions & 0 deletions regression-tests/pure2-enum.cpp2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,17 @@ main: () = {
x: skat_game = skat_game::clubs;
x2 := skat_game::diamonds;
x2 = x;
x3 := skat_game::from_string("hearts");
x4 := skat_game::from_code("skat_game::hearts");

// if x == 9 { } // error, can't compare skat_game and integer
// if x == rgb::red { } // error, can't compare skat_game and rgb color

std::cout << "x.to_string() is (x.to_string())$\n";
std::cout << "x2.to_string() is (x2.to_string())$\n";
std::cout << "x3.to_string() is (x3.to_string())$\n";
std::cout << "x3.to_code() is (x3.to_code())$\n";
std::cout << "x4.to_string() is (x3.to_string())$\n";

std::cout << "with if else: ";
if x == skat_game::diamonds { // ok, can compare two skat_games
Expand Down Expand Up @@ -117,4 +122,14 @@ main: () = {
is (cpp2::has_flags(f2)) = "includes all f2's flags ('cached' and 'current')";
is _ = "something else";
} << "\n";

f_from_string := file_attributes::from_string("cached_and_current");
std::cout << "f_from_string is " << f_from_string.to_string() << "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

It bothers me slightly that both of these:

file_attributes::to_string(file_attributes::from_string("cached_and_current"));
(file_attributes::cached | file_attributes::current).to_string()

result in this:

"(cached, current, cached_and_current)"

I'm wondering if a bitwise enum to_string() should be a single combined value if it exactly matches a combined value, and then if it doesn't, only include the bare flags.

That would make the first ones result in "cached_and_current". Then (file_attributes::current | file:attributes.obsolete).to_string() would be (current, obsolete).

Also, should to_string() include file_attributes::?

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks!

It bothers me slightly that both of these:

file_attributes::to_string(file_attributes::from_string("cached_and_current"));
(file_attributes::cached | file_attributes::current).to_string()

result in this:

"(cached, current, cached_and_current)"

I'm wondering if a bitwise enum to_string() should be a single combined value if it exactly matches a combined value, and then if it doesn't, only include the bare flags.

What I've been doing with to_string is emitting all of the named values that are matched. That way it's consistent with has.

Also, should to_string() include file_attributes::?

to_string has a this parameter, from_string doesn't so it needs the qualification.

Copy link
Contributor

@gregmarr gregmarr Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What I've been doing with to_string is emitting all of the named values that are matched. That way it's consistent with has.

My "nightmare scenario" is something like the creating an enum for the file permissions in <sys/stat.h>, and then doing ALLPERMS.to_string() and getting this:

"(S_ISUID, S_ISGID, S_ISTXT, S_ISVTX, S_IRWXU, S_IRUSR, S_IWUSR, S_IXUSR, 
S_IREAD, S_IWRITE, S_IEXEC, S_IRWXG, S_IRGRP, S_IWGRP, S_IXGRP, 
S_IRWXO, S_IROTH, S_IWOTH, S_IXOTH, ACCESSPERMS, ALLPERMS, 
DEFFILEMODE)"

to_string has a this parameter, from_string doesn't so it needs the qualification.

To clarify, I meant in the output string. It was just a thought, not for computer consumption, but more for human consumption. I was thinking that the user always has to spell it as file_attributes::current, unless they're inside the class, so would it make sense for to_string() to produce that? That would make from_string() more difficult, I guess.

Related to that, would it make sense to use | instead of , so that it makes actually usable as code?

Copy link
Contributor

Choose a reason for hiding this comment

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

Related to that, would it make sense to use | instead of , so that it makes actually usable as code?

👍🏻 I was thinking of this myself when I used flag_enums a while ago.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks! I just pushed a commit to change , to | for flag enums.

Allowing qualification is doable but just makes reading and parsing harder. A poor person's way to make it a qualified string would be to prepend flag_name:: and then replace every | with | flag_name::... perhaps it's worth adding a to_string_qualified that just calls to_string and then does that, and a from_string_qualified that replaces all flag_name:: with nothing and calls from_string...?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, it could be a thing. Python for example has __repr__ and __str__ which cover both of these use-cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if there are use cases here, just things that came to mind as I was reading it.

Maybe the "a single combined value if it exactly matches a combined value, and then if it doesn't, only include the bare flags" would be a separate function and it could have a qualified option?


f_from_string = file_attributes::from_string("(current, obsolete)");
std::cout << "f_from_string is " << f_from_string.to_string() << "\n";
std::cout << "f_from_string.to_code() is " << f_from_string.to_code() << "\n";

f_from_string = file_attributes::from_code("(file_attributes::cached | file_attributes::obsolete)");
std::cout << "f_from_string is " << f_from_string.to_string() << "\n";
}
7 changes: 7 additions & 0 deletions regression-tests/test-results/clang-12-c++20/pure2-enum.cpp.execution
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
x.to_string() is clubs
x2.to_string() is clubs
x3.to_string() is hearts
x3.to_code() is skat_game::hearts
x4.to_string() is hearts
with if else: clubs
with inspect: clubs

Expand Down Expand Up @@ -27,3 +30,7 @@ f is (f2) is false
f2 is (f ) is false
(f & f2) == f2 is true
inspecting f: includes all f2's flags ('cached' and 'current')
f_from_string is (cached, current, cached_and_current)
f_from_string is (current, obsolete)
f_from_string.to_code() is (file_attributes::current | file_attributes::obsolete)
f_from_string is (cached, obsolete)
7 changes: 7 additions & 0 deletions regression-tests/test-results/gcc-10-c++20/pure2-enum.cpp.execution
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
x.to_string() is clubs
x2.to_string() is clubs
x3.to_string() is hearts
x3.to_code() is skat_game::hearts
x4.to_string() is hearts
with if else: clubs
with inspect: clubs

Expand Down Expand Up @@ -27,3 +30,7 @@ f is (f2) is false
f2 is (f ) is false
(f & f2) == f2 is true
inspecting f: includes all f2's flags ('cached' and 'current')
f_from_string is (cached, current, cached_and_current)
f_from_string is (current, obsolete)
f_from_string.to_code() is (file_attributes::current | file_attributes::obsolete)
f_from_string is (cached, obsolete)
7 changes: 7 additions & 0 deletions regression-tests/test-results/gcc-14-c++2b/pure2-enum.cpp.execution
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
x.to_string() is clubs
x2.to_string() is clubs
x3.to_string() is hearts
x3.to_code() is skat_game::hearts
x4.to_string() is hearts
with if else: clubs
with inspect: clubs

Expand Down Expand Up @@ -27,3 +30,7 @@ f is (f2) is false
f2 is (f ) is false
(f & f2) == f2 is true
inspecting f: includes all f2's flags ('cached' and 'current')
f_from_string is (cached, current, cached_and_current)
f_from_string is (current, obsolete)
f_from_string.to_code() is (file_attributes::current | file_attributes::obsolete)
f_from_string is (cached, obsolete)
7 changes: 7 additions & 0 deletions regression-tests/test-results/msvc-2022-c++latest/pure2-enum.cpp.execution
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
x.to_string() is clubs
x2.to_string() is clubs
x3.to_string() is hearts
x3.to_code() is skat_game::hearts
x4.to_string() is hearts
with if else: clubs
with inspect: clubs

Expand Down Expand Up @@ -27,3 +30,7 @@ f is (f2) is false
f2 is (f ) is false
(f & f2) == f2 is true
inspecting f: includes all f2's flags ('cached' and 'current')
f_from_string is (cached, current, cached_and_current)
f_from_string is (current, obsolete)
f_from_string.to_code() is (file_attributes::current | file_attributes::obsolete)
f_from_string is (cached, obsolete)
Loading
Loading