[PATCH] D78085: [AST] Fix recovery-expr crash on invalid aligned attr.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang/lib/AST/DeclBase.cpp:400
+  for (; I != E; ++I) {
+    if (!I->isAlignmentDependent())
+      Align = std::max(Align, I->getAlignment(Ctx));
----------------
sammccall wrote:
> This doesn't seem great - previously if e.g. codegen ends up needing the alignment of a dependent decl somehow, then an assert will catch that programming error.
> Now it'll be silently swallowed. Seems better to check isAlignmentErrorDependent() here (you'd need to add that function) and continue to assert in other dependent cases.
ah, right. I think we should probably fix other similar places (where I added FIXMEs). We don't have problems at the moment -- because the recovery-expr is dependent, and they have been bailed out by isAlignmentDependent(). If we start capture the concrete type for recovery-expr, the `isAlignmentDependent` bailout will fail.


The `isAlignmentDependent` seems a bit confusing now, what does the `dependent` mean? if we generalize it, it could mean the attr depends on a template parameter (type, value, instantiation) or an error, in this sense `isAlignmentDependent` should cover the error-dependent case. I think here it only indicates the type-, value-, instantiation- dependent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78085