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

[hokein]

Summary

crash stack:

lang: tools/clang/include/clang/AST/AttrImpl.inc:1490: unsigned int clang::AlignedAttr::getAlignment(clang::ASTContext &) const: Assertion `!isAlignmentDependent()' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: ./bin/clang -cc1 -std=c++1y -ast-dump -frecovery-ast -fcxx-exceptions /tmp/t4.cpp
1.      /tmp/t4.cpp:3:31: current parser token ';'
 #0 0x0000000002530cff llvm::sys::PrintStackTrace(llvm::raw_ostream&) llvm-project/llvm/lib/Support/Unix/Signals.inc:564:13
 #1 0x000000000252ee30 llvm::sys::RunSignalHandlers() llvm-project/llvm/lib/Support/Signals.cpp:69:18
 #2 0x000000000253126c SignalHandler(int) llvm-project/llvm/lib/Support/Unix/Signals.inc:396:3
 #3 0x00007f86964d0520 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x13520)
 #4 0x00007f8695f9ff61 raise /build/glibc-oCLvUT/glibc-2.29/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x00007f8695f8b535 abort /build/glibc-oCLvUT/glibc-2.29/stdlib/abort.c:81:7
 #6 0x00007f8695f8b40f _nl_load_domain /build/glibc-oCLvUT/glibc-2.29/intl/loadmsgcat.c:1177:9
 #7 0x00007f8695f98b92 (/lib/x86_64-linux-gnu/libc.so.6+0x32b92)
 #8 0x0000000004503d9f llvm::APInt::getZExtValue() const llvm-project/llvm/include/llvm/ADT/APInt.h:1623:5
 #9 0x0000000004503d9f clang::AlignedAttr::getAlignment(clang::ASTContext&) const llvm-project/build/tools/clang/include/clang/AST/AttrImpl.inc:1492:0

[sammccall]

clang/lib/AST/DeclBase.cpp

400

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.

[hokein]

Use isAlignmentErrorDependent.

[hokein]

clang/lib/AST/DeclBase.cpp

400

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.

[sammccall]

clang/lib/AST/ComputeDependence.cpp

74

maybe just do this unless it causes regressions?

clang/lib/Sema/SemaDecl.cpp

2434 ↗

(On Diff #257638)

I actually don't think we should add this FIXME.
Currently error dependence implies dependence. If we break this invariant, there are going to be hundreds of places that check for dependence and need to be updated. We haven't annotated them all with fixmes.

[sammccall]

LG with comments

[hokein]

address comments.

clang/lib/AST/ComputeDependence.cpp

ExprDependence clang::computeDependence(UnaryExprOrTypeTraitExpr *E) {
   const ValueDecl *D = nullptr;
   if (const auto *DRE = dyn_cast<DeclRefExpr>(NoParens))
     D = DRE->getDecl();
   else if (const auto *ME = dyn_cast<MemberExpr>(NoParens))
     D = ME->getMemberDecl();
   if (!D)
     return Deps;
   for (const auto *I : D->specific_attrs<AlignedAttr>()) {
+    if (I->isAlignmentErrorDependent())
+      Deps |= ExprDependence::Error;
     if (I->isAlignmentDependent())
-      return Deps | ExprDependence::ValueInstantiation;
+      Deps |= ExprDependence::ValueInstantiation;
   }
   return Deps;
 }
 
 ExprDependence clang::computeDependence(ArraySubscriptExpr *E) {
   return E->getLHS()->getDependence() | E->getRHS()->getDependence();
 }
 

clang/lib/AST/DeclBase.cpp

 unsigned Decl::getMaxAlignment() const {
   if (!hasAttrs())
     return 0;
 
   unsigned Align = 0;
   const AttrVec &V = getAttrs();
   ASTContext &Ctx = getASTContext();
   specific_attr_iterator<AlignedAttr> I(V.begin()), E(V.end());
-  for (; I != E; ++I)
+  for (; I != E; ++I) {
+    if (!I->isAlignmentErrorDependent())
       Align = std::max(Align, I->getAlignment(Ctx));
+  }
   return Align;
 }
 
 bool Decl::isUsed(bool CheckUsedAttr) const {
   const Decl *CanonD = getCanonicalDecl();
   if (CanonD->Used)
     return true;
 

clang/test/AST/ast-dump-recovery.cpp

 // CHECK-NEXT: `-CallExpr {{.*}} contains-errors
 // CHECK-NEXT:  |-RecoveryExpr {{.*}} contains-errors
 // CHECK-NEXT:  | `-DeclRefExpr {{.*}} 'foo'
 // CHECK-NEXT:  `-DeclRefExpr {{.*}} 'x'
 struct Foo {} foo;
 void test(int x) {
   foo.abc;
   foo->func(x);
-}
+}
+
+// CHECK:     |-AlignedAttr {{.*}} alignas
+// CHECK-NEXT:| `-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
+struct alignas(invalid()) Aligned {};

clang/test/SemaCXX/invalid-aligned-attr.cpp

+// RUN: %clang_cc1 -frecovery-ast -verify %s
+// RUN: %clang_cc1 -verify %s
+
+struct alignas(invalid()) Foo {}; // expected-error {{use of undeclared identifier}}
+
+constexpr int k = alignof(Foo);

clang/utils/TableGen/ClangAttrEmitter.cpp

namespace {
   class AlignedArgument : public Argument {
   public:
     AlignedArgument(const Record &Arg, StringRef Attr)
       : Argument(Arg, Attr)
     {}
 
     void writeAccessors(raw_ostream &OS) const override {
       OS << "  bool is" << getUpperName() << "Dependent() const;\n";
+      OS << "  bool is" << getUpperName() << "ErrorDependent() const;\n";
 
       OS << "  unsigned get" << getUpperName() << "(ASTContext &Ctx) const;\n";
 
       OS << "  bool is" << getUpperName() << "Expr() const {\n";
       OS << "    return is" << getLowerName() << "Expr;\n";
       OS << "  }\n";
 
       OS << "  Expr *get" << getUpperName() << "Expr() const {\n";
void writeAccessorDefinitions(raw_ostream &OS) const override {
       OS << "    return " << getLowerName() << "Expr && (" << getLowerName()
          << "Expr->isValueDependent() || " << getLowerName()
          << "Expr->isTypeDependent());\n";
       OS << "  else\n";
       OS << "    return " << getLowerName()
          << "Type->getType()->isDependentType();\n";
       OS << "}\n";
 
+      OS << "bool " << getAttrName() << "Attr::is" << getUpperName()
+         << "ErrorDependent() const {\n";
+      OS << "  if (is" << getLowerName() << "Expr)\n";
+      OS << "    return " << getLowerName() << "Expr && " << getLowerName()
+         << "Expr->containsErrors();\n";
+      OS << "  return " << getLowerName()
+         << "Type->getType()->containsErrors();\n";
+      OS << "}\n";
+
       // FIXME: Do not do the calculation here
       // FIXME: Handle types correctly
       // A null pointer means maximum alignment
       OS << "unsigned " << getAttrName() << "Attr::get" << getUpperName()
          << "(ASTContext &Ctx) const {\n";
       OS << "  assert(!is" << getUpperName() << "Dependent());\n";
       OS << "  if (is" << getLowerName() << "Expr)\n";
       OS << "    return " << getLowerName() << "Expr ? " << getLowerName()