[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Mariya Podchishchaeva via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4a792e06e8e7: [clang] Fix new-expression with 
elaborated-type-specifier (authored by Fznamznon).

Changed prior to commit:
  https://reviews.llvm.org/D153857?vs=535779=536702#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
  clang/test/CXX/drs/dr19xx.cpp
  clang/test/CXX/drs/dr21xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Parser/cxx11-type-specifier.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -12653,7 +12653,7 @@
 https://cplusplus.github.io/CWG/issues/2141.html;>2141
 CD4
 Ambiguity in new-expression with elaborated-type-specifier
-Unknown
+Clang 17
   
   
 https://cplusplus.github.io/CWG/issues/2142.html;>2142
Index: clang/test/Parser/cxx11-type-specifier.cpp
===
--- clang/test/Parser/cxx11-type-specifier.cpp
+++ clang/test/Parser/cxx11-type-specifier.cpp
@@ -13,10 +13,8 @@
   } catch (constexpr int) { // expected-error{{type name does not allow constexpr}}
   }
 
-  // These parse as type definitions, not as type references with braced
-  // initializers. Sad but true...
-  (void) new struct S {}; // expected-error{{'S' cannot be defined in a type specifier}}
-  (void) new enum E { e }; // expected-error{{'E' cannot be defined in a type specifier}}
+  (void) new struct S {};
+  (void) new enum E { e };
 }
 
 // And for trailing-type-specifier-seq
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1078,9 +1078,10 @@
 (void)const_cast(0); // expected-error {{cannot be defined in a type specifier}}
 (void)sizeof(struct F*);
 (void)sizeof(struct F{}*); // expected-error {{cannot be defined in a type specifier}}
-(void)new struct G*;
-(void)new struct G{}*; // expected-error {{cannot be defined in a type specifier}}
+(void)new struct G*; // expected-note {{forward}}
+(void)new struct G{}*; // expected-error {{incomplete}}
 #if __cplusplus >= 201103L
+// expected-error@-2 {{expected expression}}
 (void)alignof(struct H*);
 (void)alignof(struct H{}*); // expected-error {{cannot be defined in a type specifier}}
 #endif
Index: clang/test/CXX/drs/dr21xx.cpp
===
--- clang/test/CXX/drs/dr21xx.cpp
+++ clang/test/CXX/drs/dr21xx.cpp
@@ -120,6 +120,31 @@
 #endif
 }
 
+namespace dr2141 { // dr2141: 17
+struct A{};
+
+template 
+struct B{};
+
+void foo() {
+  struct A *b = (1 == 1) ? new struct A : new struct A;
+  struct S *a = (1 == 1) ? new struct S : new struct S; // expected-error 2{{allocation of incomplete type}} // expected-note 2{{forward}}
+
+#if __cplusplus >= 201103L
+  A *aa = new struct A{};
+  B *bb = new struct B{};
+  (void)new struct C{}; // expected-error {{allocation of incomplete type }} // expected-note {{forward}}
+
+  struct A *c = (1 == 1) ? new struct A {} : new struct A {};
+
+  alignof(struct D{}); // expected-error {{cannot be defined in a type specifier}}
+#endif
+
+  sizeof(struct E{}); // expected-error {{cannot be defined in a type specifier}}
+
+}
+}
+
 namespace dr2157 { // dr2157: 11
 #if __cplusplus >= 201103L
   enum E : int;
Index: clang/test/CXX/drs/dr19xx.cpp
===
--- clang/test/CXX/drs/dr19xx.cpp
+++ clang/test/CXX/drs/dr19xx.cpp
@@ -194,7 +194,7 @@
 enum E : int {1}; // expected-error {{expected identifier}} (not bit-field)
   };
   auto *p1 = new enum E : int; // expected-error {{only permitted as a standalone declaration}}
-  auto *p2 = new enum F : int {}; // expected-error {{cannot be defined in a type specifier}}
+  auto *p2 = new enum F : int {}; // expected-error {{only permitted as a standalone declaration}}
   auto *p3 = true ? new enum G : int {}; // expected-error {{forward reference}} expected-error {{incomplete}} expected-note {{declaration}}
   auto h() -> enum E : int {}; // expected-error {{only permitted as a standalone declaration}}
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
@@ -21,8 +21,8 @@
   for (struct S { S(int) {} } s : Undeclared); // expected-error{{types may not be defined in a for 

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you!




Comment at: clang/docs/ReleaseNotes.rst:552
   (`#63503 `_)
+- Fixed parsing of elaborated type specifier inside of a new expression
+  (`#34341 `_)





Comment at: clang/include/clang/Parse/Parser.h:2221
+DSC_association, // A _Generic selection expression's type association
+DSC_new  // C++ new operator
   };

Fznamznon wrote:
> aaron.ballman wrote:
> > Adding the comma so the next person doesn't have to, and realigning to the 
> > usual indentation
> Added comma, but the indentation is what clang-format gave me. I'm afraid I 
> won't pass the pre-commit with different .
Ah, we don't care if pre-commit CI fails because of clang-format due to 
following the surrounding formatting style. That said, this formatting is 
already inconsistent (I hadn't noticed the first three are at a different 
indentation level as well), so this is fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 535779.
Fznamznon added a comment.

Move test cases to CXX/drs/dr21xx.cpp, update DR status page, add alignof/sizeof
cases, rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
  clang/test/CXX/drs/dr19xx.cpp
  clang/test/CXX/drs/dr21xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Parser/cxx11-type-specifier.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -12653,7 +12653,7 @@
 https://cplusplus.github.io/CWG/issues/2141.html;>2141
 CD4
 Ambiguity in new-expression with elaborated-type-specifier
-Unknown
+Clang 17
   
   
 https://cplusplus.github.io/CWG/issues/2142.html;>2142
Index: clang/test/Parser/cxx11-type-specifier.cpp
===
--- clang/test/Parser/cxx11-type-specifier.cpp
+++ clang/test/Parser/cxx11-type-specifier.cpp
@@ -13,10 +13,8 @@
   } catch (constexpr int) { // expected-error{{type name does not allow constexpr}}
   }
 
-  // These parse as type definitions, not as type references with braced
-  // initializers. Sad but true...
-  (void) new struct S {}; // expected-error{{'S' cannot be defined in a type specifier}}
-  (void) new enum E { e }; // expected-error{{'E' cannot be defined in a type specifier}}
+  (void) new struct S {};
+  (void) new enum E { e };
 }
 
 // And for trailing-type-specifier-seq
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1078,9 +1078,10 @@
 (void)const_cast(0); // expected-error {{cannot be defined in a type specifier}}
 (void)sizeof(struct F*);
 (void)sizeof(struct F{}*); // expected-error {{cannot be defined in a type specifier}}
-(void)new struct G*;
-(void)new struct G{}*; // expected-error {{cannot be defined in a type specifier}}
+(void)new struct G*; // expected-note {{forward}}
+(void)new struct G{}*; // expected-error {{incomplete}}
 #if __cplusplus >= 201103L
+// expected-error@-2 {{expected expression}}
 (void)alignof(struct H*);
 (void)alignof(struct H{}*); // expected-error {{cannot be defined in a type specifier}}
 #endif
Index: clang/test/CXX/drs/dr21xx.cpp
===
--- clang/test/CXX/drs/dr21xx.cpp
+++ clang/test/CXX/drs/dr21xx.cpp
@@ -120,6 +120,31 @@
 #endif
 }
 
+namespace dr2141 { // dr2141: 17
+struct A{};
+
+template 
+struct B{};
+
+void foo() {
+  struct A *b = (1 == 1) ? new struct A : new struct A;
+  struct S *a = (1 == 1) ? new struct S : new struct S; // expected-error 2{{allocation of incomplete type}} // expected-note 2{{forward}}
+
+#if __cplusplus >= 201103L
+  A *aa = new struct A{};
+  B *bb = new struct B{};
+  (void)new struct C{}; // expected-error {{allocation of incomplete type }} // expected-note {{forward}}
+
+  struct A *c = (1 == 1) ? new struct A {} : new struct A {};
+
+  alignof(struct D{}); // expected-error {{cannot be defined in a type specifier}}
+#endif
+
+  sizeof(struct E{}); // expected-error {{cannot be defined in a type specifier}}
+
+}
+}
+
 namespace dr2157 { // dr2157: 11
 #if __cplusplus >= 201103L
   enum E : int;
Index: clang/test/CXX/drs/dr19xx.cpp
===
--- clang/test/CXX/drs/dr19xx.cpp
+++ clang/test/CXX/drs/dr19xx.cpp
@@ -194,7 +194,7 @@
 enum E : int {1}; // expected-error {{expected identifier}} (not bit-field)
   };
   auto *p1 = new enum E : int; // expected-error {{only permitted as a standalone declaration}}
-  auto *p2 = new enum F : int {}; // expected-error {{cannot be defined in a type specifier}}
+  auto *p2 = new enum F : int {}; // expected-error {{only permitted as a standalone declaration}}
   auto *p3 = true ? new enum G : int {}; // expected-error {{forward reference}} expected-error {{incomplete}} expected-note {{declaration}}
   auto h() -> enum E : int {}; // expected-error {{only permitted as a standalone declaration}}
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
@@ -21,8 +21,8 @@
   for (struct S { S(int) {} } s : Undeclared); // expected-error{{types may not be defined in a for range declaration}}
// expected-error@-1{{use of undeclared identifier 'Undeclared'}}
 
-  new struct T {}; 

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D153857#4455480 , @Fznamznon wrote:

>> MSVC rejects the first but accepts the second which I think is wrong but 
>> worth testing:
>>
>> alignof(struct B {});
>> sizeof(struct B{});
>
> These don't have `new` so they are not affected by the patch ATM.
> But why MSVC wrong? Operand of `alignof` is a `type-id` which is not 
> `defining-type-id` meaning it shouldn't be parsed as type definition. So, 
> `struct B{}` there is a reference to `struct B` with braced initializer, i.e. 
> object creation. That is not a `type-id`, right? On the opposite, `sizeof` 
> can accept either `type-id` or an expression, I guess this is why MSVC 
> accepts.

new is special b/c it is has `new-initializer` after the `type-id` so that can 
be an initialization: https://eel.is/c++draft/expr.new#nt:new-expression in 
`sizeof` it is a definition which is not allowed and so it should be rejected.

if you do:

  struct B{ int x; };
  
  void f() {
 struct B{};
  }

That is a definition in `f`: https://godbolt.org/z/38eM6z17K


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/include/clang/Parse/Parser.h:2221
+DSC_association, // A _Generic selection expression's type association
+DSC_new  // C++ new operator
   };

aaron.ballman wrote:
> Adding the comma so the next person doesn't have to, and realigning to the 
> usual indentation
Added comma, but the indentation is what clang-format gave me. I'm afraid I 
won't pass the pre-commit with different .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 535317.
Fznamznon added a comment.

Add comma and test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
  clang/test/CXX/drs/dr19xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Parser/cxx11-type-specifier.cpp

Index: clang/test/Parser/cxx11-type-specifier.cpp
===
--- clang/test/Parser/cxx11-type-specifier.cpp
+++ clang/test/Parser/cxx11-type-specifier.cpp
@@ -7,16 +7,21 @@
 };
 enum E { e };
 
+template 
+struct Template { };
+
 void f() {
   try {
 (void) new constexpr int; // expected-error{{type name does not allow constexpr}}
   } catch (constexpr int) { // expected-error{{type name does not allow constexpr}}
   }
 
-  // These parse as type definitions, not as type references with braced
-  // initializers. Sad but true...
-  (void) new struct S {}; // expected-error{{'S' cannot be defined in a type specifier}}
-  (void) new enum E { e }; // expected-error{{'E' cannot be defined in a type specifier}}
+  (void) new struct S {};
+  (void) new enum E { e };
+  struct S *a = (1 == 1) ? new struct S : new struct S;
+  struct A *b = (1 == 1) ? new struct A : new struct A; // expected-error 2{{allocation of incomplete type}} // expected-note 2{{forward}}
+
+  (void) new Template {};
 }
 
 // And for trailing-type-specifier-seq
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1078,9 +1078,10 @@
 (void)const_cast(0); // expected-error {{cannot be defined in a type specifier}}
 (void)sizeof(struct F*);
 (void)sizeof(struct F{}*); // expected-error {{cannot be defined in a type specifier}}
-(void)new struct G*;
-(void)new struct G{}*; // expected-error {{cannot be defined in a type specifier}}
+(void)new struct G*; // expected-note {{forward}}
+(void)new struct G{}*; // expected-error {{incomplete}}
 #if __cplusplus >= 201103L
+// expected-error@-2 {{expected expression}}
 (void)alignof(struct H*);
 (void)alignof(struct H{}*); // expected-error {{cannot be defined in a type specifier}}
 #endif
Index: clang/test/CXX/drs/dr19xx.cpp
===
--- clang/test/CXX/drs/dr19xx.cpp
+++ clang/test/CXX/drs/dr19xx.cpp
@@ -194,7 +194,7 @@
 enum E : int {1}; // expected-error {{expected identifier}} (not bit-field)
   };
   auto *p1 = new enum E : int; // expected-error {{only permitted as a standalone declaration}}
-  auto *p2 = new enum F : int {}; // expected-error {{cannot be defined in a type specifier}}
+  auto *p2 = new enum F : int {}; // expected-error {{only permitted as a standalone declaration}}
   auto *p3 = true ? new enum G : int {}; // expected-error {{forward reference}} expected-error {{incomplete}} expected-note {{declaration}}
   auto h() -> enum E : int {}; // expected-error {{only permitted as a standalone declaration}}
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
@@ -21,8 +21,8 @@
   for (struct S { S(int) {} } s : Undeclared); // expected-error{{types may not be defined in a for range declaration}}
// expected-error@-1{{use of undeclared identifier 'Undeclared'}}
 
-  new struct T {}; // expected-error {{'T' cannot be defined in a type specifier}}
-  new struct A {}; // expected-error {{'A' cannot be defined in a type specifier}}
+  new struct T {}; // expected-error {{allocation of incomplete type 'struct T'}} //expected-note{{forward declaration of 'T'}}
+  new struct A {};
 
   try {} catch (struct U {}) {} // expected-error {{'U' cannot be defined in a type specifier}}
 
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3231,7 +3231,7 @@
 // A new-type-id is a simplified type-id, where essentially the
 // direct-declarator is replaced by a direct-new-declarator.
 MaybeParseGNUAttributes(DeclaratorInfo);
-if (ParseCXXTypeSpecifierSeq(DS))
+if (ParseCXXTypeSpecifierSeq(DS, DeclaratorContext::CXXNew))
   DeclaratorInfo.setInvalidType(true);
 else {
   DeclaratorInfo.SetSourceRange(DS.getSourceRange());
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2981,6 

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

> What about
> template 
> struct A { };
>
>   void foo() {
> new struct A {};
>   }
>
> I think this should be allowed, right?

Yes, I think so. The patch helps to allow. Thanks, I'll add it to the test.

> MSVC rejects the first but accepts the second which I think is wrong but 
> worth testing:
>
> alignof(struct B {});
> sizeof(struct B{});

These don't have `new` so they are not affected by the patch ATM.
But why MSVC wrong? Operand of `alignof` is a `type-id` which is not 
`defining-type-id` meaning it shouldn't be parsed as type definition. So, 
`struct B{}` there is a reference to `struct B` with braced initializer, i.e. 
object creation. That is not a `type-id`, right? On the opposite, `sizeof` can 
accept either `type-id` or an expression, I guess this is why MSVC accepts.




Comment at: clang/include/clang/Parse/Parser.h:2359
 case DeclSpecContext::DSC_template_param:
+case DeclSpecContext::DSC_new:
   return ImplicitTypenameContext::Yes;

shafik wrote:
> Should this be below since `DeclaratorContext::CXXNew` was grouped w/ 
> `DSC_normal` normal before?
Not really. Since there was no `DeclSpecContext` for `new` expression, it was 
mostly handled as if it was `DSC_type_specifier`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

MSVC rejects the first but accepts the second which I think is wrong but worth 
testing:

  alignof(struct B {});
  sizeof(struct B{});

https://godbolt.org/z/153jYqaPM




Comment at: clang/include/clang/Parse/Parser.h:2359
 case DeclSpecContext::DSC_template_param:
+case DeclSpecContext::DSC_new:
   return ImplicitTypenameContext::Yes;

Should this be below since `DeclaratorContext::CXXNew` was grouped w/ 
`DSC_normal` normal before?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

What about

  template 
  struct A { };
  
void foo() {
  new struct A {};
}

I think this should be allowed, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

> With code like struct A {}; , the expressions 
> were parsed as redefining struct A and failed. However, as clarified by 
> CWG2141, new-expression cannot define a type, so both these expressions 
> should be considered as valid references to the previously declared struct A.

Sure, thank you and sorry for the confusion! I should have added `struct A {};` 
to the example in the first place.

> I think the root cause of that issue is that we don't implement DR2141 
> (https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2141); are you 
> intending to cover that DR? If so, there should be tests added to 
> clang/test/CXX/drs/dr21xx.cpp.

The original intent was to fix the bug because I wasn't confident enough that 
I'll be able to cover all possible cases by the tests. Will the simple examples 
that I'm already adding to clang/test/Parser/cxx11-type-specifier.cpp test be 
enough?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D153857#4453092 , @aaron.ballman 
wrote:

> Thank you for the fix! Just to clarify some things before diving into the 
> review too much... From the patch summary:
>
>   Expressions like
>   
>   new struct A {};
>   struct A* b = (1 == 1) ? new struct A : new struct A;
>   Were parsed as definitions of struct A and failed, however as clarified by
>   CWG2141 new-expression cannot define a type, so both these examples
>   should be considered as valid.
>
> There's a typo there -- the `new struct A{};` bit should be `struct A {};` 
> (dropping the `new`), right?
>
> I think the root cause of that issue is that we don't implement DR2141 
> (https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2141); are you 
> intending to cover that DR? If so, there should be tests added to 
> clang/test/CXX/drs/dr21xx.cpp.

With offline help from @erichkeane and @shafik I realized where my confusion 
came from here. The patch summary presumes `struct A` is already defined in 
order for us to accept those code examples. I was thrown off by the DR and the 
original issue using `struct A {};` as the first relevant line in the code 
example and was thinking there was confusion as to how the DR was resolved.

How about the summary be changed to something along the lines of:

With code like `struct A {}; `, the expressions 
were parsed as redefining `struct A` and failed. However, as clarified by 
CWG2141, new-expression cannot define a type, so both these expressions should 
be considered as valid references to the previously declared `struct A`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for the fix! Just to clarify some things before diving into the 
review too much... From the patch summary:

  Expressions like
  
  new struct A {};
  struct A* b = (1 == 1) ? new struct A : new struct A;
  Were parsed as definitions of struct A and failed, however as clarified by
  CWG2141 new-expression cannot define a type, so both these examples
  should be considered as valid.

There's a typo there -- the `new struct A{};` bit should be `struct A {};` 
(dropping the `new`), right?

I think the root cause of that issue is that we don't implement DR2141 
(https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2141); are you 
intending to cover that DR? If so, there should be tests added to 
clang/test/CXX/drs/dr21xx.cpp.




Comment at: clang/include/clang/Parse/Parser.h:2221
+DSC_association, // A _Generic selection expression's type association
+DSC_new  // C++ new operator
   };

Adding the comma so the next person doesn't have to, and realigning to the 
usual indentation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153857/new/

https://reviews.llvm.org/D153857

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Expressions like

  new struct A {};
  struct A* b = (1 == 1) ? new struct A : new struct A;
  ``
  Were parsed as definitions of `struct A` and failed, however as clarified by
  `CWG2141` new-expression cannot define a type, so both these examples
  should be considered as valid.
  The patch adds a "new" kind context for parsing declaration specifiers in
  addition to already existing declarator context in order to track that
  the parser is inside of a new expression.
  
  Fixes https://github.com/llvm/llvm-project/issues/34341


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153857

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
  clang/test/CXX/drs/dr19xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Parser/cxx11-type-specifier.cpp

Index: clang/test/Parser/cxx11-type-specifier.cpp
===
--- clang/test/Parser/cxx11-type-specifier.cpp
+++ clang/test/Parser/cxx11-type-specifier.cpp
@@ -13,10 +13,10 @@
   } catch (constexpr int) { // expected-error{{type name does not allow constexpr}}
   }
 
-  // These parse as type definitions, not as type references with braced
-  // initializers. Sad but true...
-  (void) new struct S {}; // expected-error{{'S' cannot be defined in a type specifier}}
-  (void) new enum E { e }; // expected-error{{'E' cannot be defined in a type specifier}}
+  (void) new struct S {};
+  (void) new enum E { e };
+  struct S *a = (1 == 1) ? new struct S : new struct S;
+  struct A *b = (1 == 1) ? new struct A : new struct A; // expected-error 2{{allocation of incomplete type}} // expected-note 2{{forward}}
 }
 
 // And for trailing-type-specifier-seq
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1078,9 +1078,10 @@
 (void)const_cast(0); // expected-error {{cannot be defined in a type specifier}}
 (void)sizeof(struct F*);
 (void)sizeof(struct F{}*); // expected-error {{cannot be defined in a type specifier}}
-(void)new struct G*;
-(void)new struct G{}*; // expected-error {{cannot be defined in a type specifier}}
+(void)new struct G*; // expected-note {{forward}}
+(void)new struct G{}*; // expected-error {{incomplete}}
 #if __cplusplus >= 201103L
+// expected-error@-2 {{expected expression}}
 (void)alignof(struct H*);
 (void)alignof(struct H{}*); // expected-error {{cannot be defined in a type specifier}}
 #endif
Index: clang/test/CXX/drs/dr19xx.cpp
===
--- clang/test/CXX/drs/dr19xx.cpp
+++ clang/test/CXX/drs/dr19xx.cpp
@@ -194,7 +194,7 @@
 enum E : int {1}; // expected-error {{expected identifier}} (not bit-field)
   };
   auto *p1 = new enum E : int; // expected-error {{only permitted as a standalone declaration}}
-  auto *p2 = new enum F : int {}; // expected-error {{cannot be defined in a type specifier}}
+  auto *p2 = new enum F : int {}; // expected-error {{only permitted as a standalone declaration}}
   auto *p3 = true ? new enum G : int {}; // expected-error {{forward reference}} expected-error {{incomplete}} expected-note {{declaration}}
   auto h() -> enum E : int {}; // expected-error {{only permitted as a standalone declaration}}
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp
@@ -21,8 +21,8 @@
   for (struct S { S(int) {} } s : Undeclared); // expected-error{{types may not be defined in a for range declaration}}
// expected-error@-1{{use of undeclared identifier 'Undeclared'}}
 
-  new struct T {}; // expected-error {{'T' cannot be defined in a type specifier}}
-  new struct A {}; // expected-error {{'A' cannot be defined in a type specifier}}
+  new struct T {}; // expected-error {{allocation of incomplete type 'struct T'}} //expected-note{{forward declaration of 'T'}}
+  new struct A {};
 
   try {} catch (struct U {}) {} // expected-error {{'U' cannot be defined in a type specifier}}
 
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3231,7 +3231,7 @@
 // A new-type-id is a simplified type-id, where essentially the
 // direct-declarator is replaced by a direct-new-declarator.
 MaybeParseGNUAttributes(DeclaratorInfo);
-if (ParseCXXTypeSpecifierSeq(DS))
+