hokein added a comment.

> This makes me nervous, marking the constructor as invalid seems much safer. 
> Can you show tests it regresses?

Yeah, the first version of the patch doesn't seem to fix all crashes (X().Y 
would lead another crash)...

so if we mark the CtorDecl as invalid

1. whenever `CtorInitializer->init()->containsError`, we don't have failure 
tests
2. whenever there is any kind of errors (including the containsError case 
above) in CtorInitailizer, we have three failing tests 
(SemaCXX/constant-expression-cxx11.cpp, SemaCXX/constructor-initializer.cpp, 
SemaTemplate/constexpr-instantiate.cpp).

though 1) passes all existing tests, I think it just means current tests don't 
have enough coverage for recoveryExpr cases. But given the current state, 1) 
looks most promising -- fixes the crashes, retains broken expressions in 
CtorInitializer rather than dropping them, doesn't regress the diagnostics a 
lot (only for CtorInitializer->init()->containsError` case).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77041



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

Reply via email to