[PATCH] D80508: [AST] Fix the source range for TagDecl if there is no matched } brace.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added reviewers: sammccall, akyrtzi.
Herald added subscribers: usaxena95, kadircet, arphaman, dexonsmith, jkorous, 
ilya-biryukov.
Herald added a project: clang.

The AST is preserved when the TagDecl misses a } brace, but the source
range seems incorrect (just covers the name, not the body, because
RBraceLoc is invalid and we fallback to NameLoc). This breaks the
SelectionTree in clangd -- clangd skips the TagDecl entirely.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80508

Files:
  clang-tools-extra/clangd/unittests/SelectionTests.cpp
  clang/lib/AST/Decl.cpp
  clang/test/AST/ast-dump-invalid-brace.cpp


Index: clang/test/AST/ast-dump-invalid-brace.cpp
===================================================================
--- /dev/null
+++ clang/test/AST/ast-dump-invalid-brace.cpp
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -ast-dump 
-ast-dump-filter Test %s | FileCheck -strict-whitespace %s
+
+// CHECK: CXXRecordDecl {{.*}}:4:1, <invalid sloc>
+class Test {
+  int abc;
+// }
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4126,8 +4126,8 @@
 }
 
 SourceRange TagDecl::getSourceRange() const {
-  SourceLocation RBraceLoc = BraceRange.getEnd();
-  SourceLocation E = RBraceLoc.isValid() ? RBraceLoc : getLocation();
+  SourceLocation E = BraceRange.getBegin().isValid() ?
+                     BraceRange.getEnd() : getLocation();
   return SourceRange(getOuterLocStart(), E);
 }
 
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -388,7 +388,26 @@
         void test(S2 s2) {
           s2[[-^>]]f();
         }
-      )cpp", "DeclRefExpr"} // DeclRefExpr to the "operator->" method.
+      )cpp",
+       "DeclRefExpr"}, // DeclRefExpr to the "operator->" method.
+
+      // broken cases, missing } brace.
+      {
+          R"cpp(
+            // error-ok: AST is still valid on missing } brace.
+            class ABC {
+            [[int ^a]];
+            // }
+          )cpp",
+          "FieldDecl"},
+      {
+          R"cpp(
+            // error-ok
+            enum Color {
+            [[^Black]],
+            //}
+          )cpp",
+          "EnumConstantDecl"},
   };
   for (const Case &C : Cases) {
     trace::TestTracer Tracer;



Index: clang/test/AST/ast-dump-invalid-brace.cpp
===================================================================
--- /dev/null
+++ clang/test/AST/ast-dump-invalid-brace.cpp
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -ast-dump -ast-dump-filter Test %s | FileCheck -strict-whitespace %s
+
+// CHECK: CXXRecordDecl {{.*}}:4:1, <invalid sloc>
+class Test {
+  int abc;
+// }
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4126,8 +4126,8 @@
 }
 
 SourceRange TagDecl::getSourceRange() const {
-  SourceLocation RBraceLoc = BraceRange.getEnd();
-  SourceLocation E = RBraceLoc.isValid() ? RBraceLoc : getLocation();
+  SourceLocation E = BraceRange.getBegin().isValid() ?
+                     BraceRange.getEnd() : getLocation();
   return SourceRange(getOuterLocStart(), E);
 }
 
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -388,7 +388,26 @@
         void test(S2 s2) {
           s2[[-^>]]f();
         }
-      )cpp", "DeclRefExpr"} // DeclRefExpr to the "operator->" method.
+      )cpp",
+       "DeclRefExpr"}, // DeclRefExpr to the "operator->" method.
+
+      // broken cases, missing } brace.
+      {
+          R"cpp(
+            // error-ok: AST is still valid on missing } brace.
+            class ABC {
+            [[int ^a]];
+            // }
+          )cpp",
+          "FieldDecl"},
+      {
+          R"cpp(
+            // error-ok
+            enum Color {
+            [[^Black]],
+            //}
+          )cpp",
+          "EnumConstantDecl"},
   };
   for (const Case &C : Cases) {
     trace::TestTracer Tracer;

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