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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Argh I never submitted these comments, sorry.



================
Comment at: clang/lib/AST/Decl.cpp:4129
 SourceRange TagDecl::getSourceRange() const {
-  SourceLocation RBraceLoc = BraceRange.getEnd();
-  SourceLocation E = RBraceLoc.isValid() ? RBraceLoc : getLocation();
+  SourceLocation E = BraceRange.getBegin().isValid() ?
+                     BraceRange.getEnd() : getLocation();
----------------
I'm confused about this change.

We were using BR.end if it was valid, now we're using it if BR.start is valid. So this changes behavior in two cases:
 - BR.start is valid, BR.end is invalid: old was {getOuterLocStart(), getLocation()}. new is {getOuterLocStart(), invalid}
 - BR.end is valid, BR.start is invalid: old was {getOuterLocStart(), BraceRange.getEnd()}. new is {getOuterLocStart(), getLocation()}

These both seem worse to me, what am I missing?

Patch description refers to the first case, I wonder if this is just a bug in clangd cancelling out a bad AST (does it just mark tokens until EOF if the end location is invalid?)


================
Comment at: clang/test/AST/ast-dump-invalid-brace.cpp:6
+  int abc;
+// }
----------------
or `// missing: }`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80508