Skip to content

Commit bf77f37

Browse files
hsutterhsutter
committed
Removed ban on using the name of a type scope name elsewhere in a type
See #1373 The programmer can always write `this.` to disambiguate. This originally seemed like a useful constraint, but it creates a tension between generated code and user code. For example, see #1373, and the example foo : @enum type = { s; } which before this commit was rejected because the `@enum` type metafunction generates a function with a parameter named `s`. It seems undesirable to force generated code to uglify its names to avoid conflicts. Also, fix `@enum` for cases like this to type-scope-qualify access to the member.
1 parent 482542b commit bf77f37

File tree

6 files changed

+51
-87
lines changed

6 files changed

+51
-87
lines changed

‎regression-tests/test-results/pure2-enum.cpp‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,12 @@ constexpr auto skat_game::operator=(skat_game&& that) noexcept -> skat_game& {
161161
[[nodiscard]] auto skat_game::to_string_impl(cpp2::impl::in<std::string_view> prefix) const& -> std::string{
162162

163163
auto pref {cpp2::to_string(prefix)};
164-
if ((*this) == diamonds) {return pref + "diamonds"; }
165-
if ((*this) == hearts) {return pref + "hearts"; }
166-
if ((*this) == spades) {return pref + "spades"; }
167-
if ((*this) == clubs) {return pref + "clubs"; }
168-
if ((*this) == grand) {return pref + "grand"; }
169-
if ((*this) == null) {return cpp2::move(pref) + "null"; }
164+
if ((*this) == skat_game::diamonds) {return pref + "diamonds"; }
165+
if ((*this) == skat_game::hearts) {return pref + "hearts"; }
166+
if ((*this) == skat_game::spades) {return pref + "spades"; }
167+
if ((*this) == skat_game::clubs) {return pref + "clubs"; }
168+
if ((*this) == skat_game::grand) {return pref + "grand"; }
169+
if ((*this) == skat_game::null) {return cpp2::move(pref) + "null"; }
170170
return "invalid skat_game value";
171171
}
172172

@@ -175,16 +175,16 @@ return "invalid skat_game value";
175175
[[nodiscard]] auto skat_game::from_string(cpp2::impl::in<std::string_view> s) -> skat_game{
176176

177177
auto x {s};
178-
if ("diamonds" == x) {return diamonds; }
179-
else {if ("hearts" == x) {return hearts; }
180-
else {if ("spades" == x) {return spades; }
181-
else {if ("clubs" == x) {return clubs; }
182-
else {if ("grand" == x) {return grand; }
183-
else {if ("null" == cpp2::move(x)) {return null; }
178+
if ("diamonds" == x) {return skat_game::diamonds; }
179+
else {if ("hearts" == x) {return skat_game::hearts; }
180+
else {if ("spades" == x) {return skat_game::spades; }
181+
else {if ("clubs" == x) {return skat_game::clubs; }
182+
else {if ("grand" == x) {return skat_game::grand; }
183+
else {if ("null" == cpp2::move(x)) {return skat_game::null; }
184184
#line 1 "pure2-enum.cpp2"
185185
}}}}}
186186
CPP2_UFCS(report_violation)(cpp2::type_safety, CPP2_UFCS(c_str)(("can't convert string '" + cpp2::to_string(s) + "' to enum of type skat_game")));
187-
return diamonds;
187+
return skat_game::diamonds;
188188
}
189189

190190
[[nodiscard]] auto skat_game::from_code(cpp2::impl::in<std::string_view> s) -> skat_game{
@@ -223,8 +223,8 @@ constexpr auto janus::operator=(janus&& that) noexcept -> janus& {
223223
[[nodiscard]] auto janus::to_string_impl(cpp2::impl::in<std::string_view> prefix) const& -> std::string{
224224

225225
auto pref {cpp2::to_string(prefix)};
226-
if ((*this) == past) {return pref + "past"; }
227-
if ((*this) == future) {return cpp2::move(pref) + "future"; }
226+
if ((*this) == janus::past) {return pref + "past"; }
227+
if ((*this) == janus::future) {return cpp2::move(pref) + "future"; }
228228
return "invalid janus value";
229229
}
230230

@@ -233,12 +233,12 @@ constexpr auto janus::operator=(janus&& that) noexcept -> janus& {
233233
[[nodiscard]] auto janus::from_string(cpp2::impl::in<std::string_view> s) -> janus{
234234

235235
auto x {s};
236-
if ("past" == x) {return past; }
237-
else {if ("future" == cpp2::move(x)) {return future; }
236+
if ("past" == x) {return janus::past; }
237+
else {if ("future" == cpp2::move(x)) {return janus::future; }
238238
#line 1 "pure2-enum.cpp2"
239239
}
240240
CPP2_UFCS(report_violation)(cpp2::type_safety, CPP2_UFCS(c_str)(("can't convert string '" + cpp2::to_string(s) + "' to enum of type janus")));
241-
return past;
241+
return janus::past;
242242
}
243243

244244
[[nodiscard]] auto janus::from_code(cpp2::impl::in<std::string_view> s) -> janus{
@@ -290,10 +290,10 @@ std::string sep {};
290290
if ((*this) == none) {return "(none)"; }
291291

292292
auto pref {cpp2::to_string(prefix)};
293-
if (((*this) & cached) == cached) {ret += sep + pref + "cached";sep = separator;}
294-
if (((*this) & current) == current) {ret += sep + pref + "current";sep = separator;}
295-
if (((*this) & obsolete) == obsolete) {ret += sep + pref + "obsolete";sep = separator;}
296-
if (((*this) & cached_and_current) == cached_and_current) {ret += sep + cpp2::move(pref) + "cached_and_current";sep = separator;}
293+
if (((*this) & file_attributes::cached) == file_attributes::cached) {ret += sep + pref + "cached";sep = separator;}
294+
if (((*this) & file_attributes::current) == file_attributes::current) {ret += sep + pref + "current";sep = separator;}
295+
if (((*this) & file_attributes::obsolete) == file_attributes::obsolete) {ret += sep + pref + "obsolete";sep = separator;}
296+
if (((*this) & file_attributes::cached_and_current) == file_attributes::cached_and_current) {ret += sep + cpp2::move(pref) + "cached_and_current";sep = separator;}
297297
return cpp2::move(ret) + ")";
298298
}
299299

@@ -304,11 +304,11 @@ return cpp2::move(ret) + ")";
304304
auto ret {none};
305305
do {{
306306
for ( auto const& x : cpp2::string_util::split_string_list(s) ) {
307-
if ("cached" == x) {ret |= cached;}
308-
else {if ("current" == x) {ret |= current;}
309-
else {if ("obsolete" == x) {ret |= obsolete;}
310-
else {if ("cached_and_current" == x) {ret |= cached_and_current;}
311-
else {if ("none" == x) {ret |= none;}
307+
if ("cached" == x) {ret |= file_attributes::cached;}
308+
else {if ("current" == x) {ret |= file_attributes::current;}
309+
else {if ("obsolete" == x) {ret |= file_attributes::obsolete;}
310+
else {if ("cached_and_current" == x) {ret |= file_attributes::cached_and_current;}
311+
else {if ("none" == x) {ret |= file_attributes::none;}
312312
else {goto BREAK_outer;}
313313
#line 1 "pure2-enum.cpp2"
314314
}}}}
@@ -320,7 +320,7 @@ return ret;
320320
false
321321
);
322322
CPP2_UFCS(report_violation)(cpp2::type_safety, CPP2_UFCS(c_str)(("can't convert string '" + cpp2::to_string(s) + "' to flag_enum of type file_attributes")));
323-
return none;
323+
return file_attributes::none;
324324
}
325325

326326
[[nodiscard]] auto file_attributes::from_code(cpp2::impl::in<std::string_view> s) -> file_attributes{

‎regression-tests/test-results/version‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11

2-
cppfront compiler v0.8.2 Build A207:1527
2+
cppfront compiler v0.8.2 Build A228:1826
33
SPDX-License-Identifier Apache-2.0 WITH LLVM-exception
44
Copyright (c) 2022-2024 Herb Sutter

‎source/build.info‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
"A207:1527"
1+
"A228:1826"

‎source/reflect.h‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3585,13 +3585,13 @@ std::string to_string_impl{" to_string_impl: (this, prefix: std::string_view"
35853585
if (e.name != "_") {// ignore unnamed values
35863586
if (bitwise) {
35873587
if (e.name != "none") {
3588-
to_string_impl += " if (this & " + cpp2::to_string(e.name) + ") == " + cpp2::to_string(e.name) + " { "
3588+
to_string_impl += " if (this & " + cpp2::to_string(CPP2_UFCS(name)(t)) + "::" + cpp2::to_string(e.name) + ") == " + cpp2::to_string(CPP2_UFCS(name)(t)) + "::" + cpp2::to_string(e.name) + " { "
35893589
"ret += sep + pref + \"" + cpp2::to_string(e.name) + "\"; sep = separator; "
35903590
"}\n";
35913591
}
35923592
}
35933593
else {
3594-
to_string_impl += " if this == " + cpp2::to_string(e.name) + " { return pref + \"" + cpp2::to_string(e.name) + "\"; }\n";
3594+
to_string_impl += " if this == " + cpp2::to_string(CPP2_UFCS(name)(t)) + "::" + cpp2::to_string(e.name) + " { return pref + \"" + cpp2::to_string(e.name) + "\"; }\n";
35953595
}
35963596
}
35973597
}
@@ -3646,7 +3646,7 @@ std::string_view else_{""};
36463646
#line 1632 "reflect.h2"
36473647
for (
36483648
auto const& e : cpp2::move(enumerators) ) {
3649-
from_string += " " + cpp2::to_string(else_) + "if \"" + cpp2::to_string(e.name) + "\" == x { " + cpp2::to_string(combine_op) + " " + cpp2::to_string(e.name) + "; }\n";
3649+
from_string += " " + cpp2::to_string(else_) + "if \"" + cpp2::to_string(e.name) + "\" == x { " + cpp2::to_string(combine_op) + " " + cpp2::to_string(CPP2_UFCS(name)(t)) + "::" + cpp2::to_string(e.name) + "; }\n";
36503650
else_ = "else ";
36513651
}
36523652
}
@@ -3660,7 +3660,7 @@ std::string_view else_{""};
36603660
}
36613661

36623662
from_string += " cpp2::type_safety.report_violation( (\"can't convert string '\" + cpp2::to_string(s) + \"' to " + cpp2::to_string(cpp2::move(prefix)) + "enum of type " + cpp2::to_string(CPP2_UFCS(name)(t)) + "\").c_str() );\n"
3663-
" return " + cpp2::to_string(cpp2::move(default_value)) + ";\n"
3663+
" return " + cpp2::to_string(CPP2_UFCS(name)(t)) + "::" + cpp2::to_string(cpp2::move(default_value)) + ";\n"
36643664
" }\n\n";
36653665

36663666
CPP2_UFCS(add_member)(t, cpp2::move(from_string));
@@ -4362,12 +4362,12 @@ std::string sep {};
43624362
if ((*this) == none) {return "(none)"; }
43634363

43644364
auto pref {cpp2::to_string(prefix)};
4365-
if (((*this) & case_insensitive) == case_insensitive) {ret += sep + pref + "case_insensitive";sep = separator;}
4366-
if (((*this) & multiple_lines) == multiple_lines) {ret += sep + pref + "multiple_lines";sep = separator;}
4367-
if (((*this) & single_line) == single_line) {ret += sep + pref + "single_line";sep = separator;}
4368-
if (((*this) & no_group_captures) == no_group_captures) {ret += sep + pref + "no_group_captures";sep = separator;}
4369-
if (((*this) & perl_code_syntax) == perl_code_syntax) {ret += sep + pref + "perl_code_syntax";sep = separator;}
4370-
if (((*this) & perl_code_syntax_in_classes) == perl_code_syntax_in_classes) {ret += sep + cpp2::move(pref) + "perl_code_syntax_in_classes";sep = separator;}
4365+
if (((*this) & expression_flags::case_insensitive) == expression_flags::case_insensitive) {ret += sep + pref + "case_insensitive";sep = separator;}
4366+
if (((*this) & expression_flags::multiple_lines) == expression_flags::multiple_lines) {ret += sep + pref + "multiple_lines";sep = separator;}
4367+
if (((*this) & expression_flags::single_line) == expression_flags::single_line) {ret += sep + pref + "single_line";sep = separator;}
4368+
if (((*this) & expression_flags::no_group_captures) == expression_flags::no_group_captures) {ret += sep + pref + "no_group_captures";sep = separator;}
4369+
if (((*this) & expression_flags::perl_code_syntax) == expression_flags::perl_code_syntax) {ret += sep + pref + "perl_code_syntax";sep = separator;}
4370+
if (((*this) & expression_flags::perl_code_syntax_in_classes) == expression_flags::perl_code_syntax_in_classes) {ret += sep + cpp2::move(pref) + "perl_code_syntax_in_classes";sep = separator;}
43714371
return cpp2::move(ret) + ")";
43724372
}
43734373

@@ -4378,13 +4378,13 @@ return cpp2::move(ret) + ")";
43784378
auto ret {none};
43794379
do {{
43804380
for ( auto const& x : cpp2::string_util::split_string_list(s) ) {
4381-
if ("case_insensitive" == x) {ret |= case_insensitive;}
4382-
else {if ("multiple_lines" == x) {ret |= multiple_lines;}
4383-
else {if ("single_line" == x) {ret |= single_line;}
4384-
else {if ("no_group_captures" == x) {ret |= no_group_captures;}
4385-
else {if ("perl_code_syntax" == x) {ret |= perl_code_syntax;}
4386-
else {if ("perl_code_syntax_in_classes" == x) {ret |= perl_code_syntax_in_classes;}
4387-
else {if ("none" == x) {ret |= none;}
4381+
if ("case_insensitive" == x) {ret |= expression_flags::case_insensitive;}
4382+
else {if ("multiple_lines" == x) {ret |= expression_flags::multiple_lines;}
4383+
else {if ("single_line" == x) {ret |= expression_flags::single_line;}
4384+
else {if ("no_group_captures" == x) {ret |= expression_flags::no_group_captures;}
4385+
else {if ("perl_code_syntax" == x) {ret |= expression_flags::perl_code_syntax;}
4386+
else {if ("perl_code_syntax_in_classes" == x) {ret |= expression_flags::perl_code_syntax_in_classes;}
4387+
else {if ("none" == x) {ret |= expression_flags::none;}
43884388
else {goto BREAK_outer;}
43894389
#line 1 "reflect.h2"
43904390
}}}}}}

‎source/reflect.h2‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,13 +1577,13 @@ basic_enum: (
15771577
if e.name != "_" { // ignore unnamed values
15781578
if bitwise {
15791579
if e.name != "none" {
1580-
to_string_impl += " if (this & (e.name)$) == (e.name)$ { "
1580+
to_string_impl += " if (this & (t.name())$::(e.name)$) == (t.name())$::(e.name)$ { "
15811581
"ret += sep + pref + \"(e.name)$\"; sep = separator; "
15821582
"}\n";
15831583
}
15841584
}
15851585
else {
1586-
to_string_impl += " if this == (e.name)$ { return pref + \"(e.name)$\"; }\n";
1586+
to_string_impl += " if this == (t.name())$::(e.name)$ { return pref + \"(e.name)$\"; }\n";
15871587
}
15881588
}
15891589
}
@@ -1631,7 +1631,7 @@ basic_enum: (
16311631
(copy else_: std::string_view = "")
16321632
for enumerators
16331633
do (e) {
1634-
from_string += " (else_)$if \"(e.name)$\" == x { (combine_op)$ (e.name)$; }\n";
1634+
from_string += " (else_)$if \"(e.name)$\" == x { (combine_op)$ (t.name())$::(e.name)$; }\n";
16351635
else_ = "else ";
16361636
}
16371637

@@ -1643,7 +1643,7 @@ basic_enum: (
16431643
}
16441644

16451645
from_string += " cpp2::type_safety.report_violation( (\"can't convert string '\" + cpp2::to_string(s) + \"' to (prefix)$enum of type (t.name())$\").c_str() );\n"
1646-
" return (default_value)$;\n"
1646+
" return (t.name())$::(default_value)$;\n"
16471647
" }\n\n";
16481648

16491649
t.add_member( from_string );

‎source/to_cpp1.h‎

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4429,34 +4429,6 @@ class cppfront
44294429
}
44304430

44314431

4432-
//-----------------------------------------------------------------------
4433-
// Within a type scope implementation, disallow declaring a name that
4434-
// is the same as (i.e., shadows) a type scope name... this is a
4435-
// convenient place to check because we have the decls stack
4436-
//
4437-
auto check_shadowing_of_type_scope_names(
4438-
declaration_node const& decl
4439-
)
4440-
-> bool
4441-
{
4442-
if (
4443-
decl.has_name() // this is a named declaration
4444-
&& !decl.has_name("this") // that's not 'this'
4445-
&& !decl.parent_is_type() // and the type isn't the direct parent
4446-
&& is_name_declared_in_current_type_scope(*decl.name())
4447-
) // and it shadows a name
4448-
{
4449-
errors.emplace_back(
4450-
decl.position(),
4451-
"a type's implementation may not declare a name that is the same as (i.e., shadows) a type scope name - for example, a type scope function's local variable may not have the same as one of the type's members"
4452-
);
4453-
return false;
4454-
}
4455-
4456-
return true;
4457-
}
4458-
4459-
44604432
//-----------------------------------------------------------------------
44614433
//
44624434
auto emit(
@@ -4475,10 +4447,6 @@ class cppfront
44754447
assert( n.declaration );
44764448
assert( !n.declaration->is_function() );
44774449

4478-
if (!check_shadowing_of_type_scope_names(*n.declaration)) {
4479-
return;
4480-
}
4481-
44824450
assert( n.declaration->identifier );
44834451
auto identifier = print_to_string( *n.declaration->identifier );
44844452
auto identifier_pos = n.position();
@@ -6032,10 +6000,6 @@ class cppfront
60326000
;
60336001
auto is_in_type = n.parent_is_type();
60346002

6035-
if (!check_shadowing_of_type_scope_names(n)) {
6036-
return;
6037-
}
6038-
60396003

60406004
// If this is a function that has multiple return values,
60416005
// first we need to emit the struct that contains the returns

0 commit comments

Comments
 (0)