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

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 257638.
hokein marked an inline comment as done.
hokein added a comment.

Use isAlignmentErrorDependent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78085

Files:
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/SemaCXX/invalid-aligned-attr.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -481,6 +481,7 @@
 
     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";
 
@@ -511,6 +512,15 @@
          << "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
Index: clang/test/SemaCXX/invalid-aligned-attr.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/invalid-aligned-attr.cpp
@@ -0,0 +1,6 @@
+// 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);
Index: clang/test/AST/ast-dump-recovery.cpp
===================================================================
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -97,4 +97,9 @@
 void test(int x) {
   foo.abc;
   foo->func(x);
-}
\ No newline at end of file
+}
+
+// CHECK:     |-AlignedAttr {{.*}} alignas
+// CHECK-NEXT:| `-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
+struct alignas(invalid()) Aligned {};
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3884,6 +3884,7 @@
   AlignedAttr *LastAlignedAttr = nullptr;
   unsigned Align = 0;
   for (auto *I : D->specific_attrs<AlignedAttr>()) {
+    // FIXME: we should bail out if I is error dependent.
     if (I->isAlignmentDependent())
       return;
     if (I->isAlignas())
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2431,6 +2431,7 @@
   AlignedAttr *OldStrictestAlignAttr = nullptr;
   unsigned OldAlign = 0;
   for (auto *I : Old->specific_attrs<AlignedAttr>()) {
+    // FIXME: we should bail out if I is error dependent.
     // FIXME: We have no way of representing inherited dependent alignments
     // in a case like:
     //   template<int A, int B> struct alignas(A) X;
@@ -2454,6 +2455,7 @@
   AlignedAttr *NewAlignasAttr = nullptr;
   unsigned NewAlign = 0;
   for (auto *I : New->specific_attrs<AlignedAttr>()) {
+    // FIXME: we should bail out if I is error dependent.
     if (I->isAlignmentDependent())
       return false;
 
Index: clang/lib/AST/DeclBase.cpp
===================================================================
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -396,8 +396,10 @@
   const AttrVec &V = getAttrs();
   ASTContext &Ctx = getASTContext();
   specific_attr_iterator<AlignedAttr> I(V.begin()), E(V.end());
-  for (; I != E; ++I)
-    Align = std::max(Align, I->getAlignment(Ctx));
+  for (; I != E; ++I) {
+    if (!I->isAlignmentErrorDependent())
+      Align = std::max(Align, I->getAlignment(Ctx));
+  }
   return Align;
 }
 
Index: clang/lib/AST/ComputeDependence.cpp
===================================================================
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -71,6 +71,7 @@
   if (!D)
     return Deps;
   for (const auto *I : D->specific_attrs<AlignedAttr>()) {
+    // FIXME: should we propagate the error bit?
     if (I->isAlignmentDependent())
       return Deps | ExprDependence::ValueInstantiation;
   }

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