hokein added inline comments.

================
Comment at: clang/test/SemaCXX/cxx11-crashes.cpp:117
+  // The missing expression makes A undeduced.
+  static constexpr auto A = ;  // expected-error {{expected expression}}
+  Foo<decltype(A)>::type B;  // The type of B is also undeduced (wrapped in 
Elaborated).
----------------
adamcz wrote:
> hokein wrote:
> > I think the root cause is that this incomplete var declaration is marked 
> > valid.
> > 
> > Looks like there are a few inconsistencies in clang:
> > 
> > ```
> > struct Bar {
> >   static constexpr auto A1; // case1: invalid
> >   static constexpr auto A2 = ; // case2: valid
> > };
> > 
> > // static var decl in global context
> > static constexpr auto A3; // case3: invalid
> > static constexpr auto A4 = ; // case4: invalid
> > ```
> > 
> > so it looks like marking case2 valid is a bug, I think we should invalidate 
> > it.
> I updated the change to mark a FieldDecl with undeduced type as invalid, thus 
> making the record invalid too. Also added a test using -ast-dump to verify 
> that both field and record are invalid, but kept the crash test too.
sorry, my comment was not clear enough -- if I understand your new change 
correctly, the change makes the field decl `Foo<decltype(A)>::type B;` but not 
the decl `A`. 

The problem here is `static constexpr auto A = ;`, this declaration (var, not 
field) is valid, which causes the field decl below `Foo<decltype(A)>::type B;` 
valid. If we invalidate the Decl `A`, then the filed decl B should be 
invalidated automatically.

leaving `A` valid is problematic -- we can't have a valid decl with an 
undeduced auto type in clang. the following case would lead another crash:

```
struct Bar {
  static constexpr auto A = ;
};
constexpr int s = sizeof(Bar::A);
```

So the solution is to make Decl `A` invalid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105478

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

Reply via email to