[PATCH] D89098: [clang] Fix returning the underlying VarDecl as top-level decl for VarTemplateDecl.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice catch, LG with readability nits



================
Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:107
 
+TEST(ParsedASTTest, VarTemplateDecl) {
+  TestTU TU;
----------------
(you could fold this into the above test if you want)


================
Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:112
+  EXPECT_THAT(AST.getLocalTopLevelDecls(),
+              ElementsAre(AllOf(DeclNamed("X"), WithTemplateArgs(""))));
+}
----------------
Is WithTemplateArgs here testing what you want (args vs params)?
Seems like it would be clearer to assert on DeclKind


================
Comment at: clang/lib/Parse/ParseDecl.cpp:2212
       // initialize it.
       ThisDecl = VT->getTemplatedDecl();
+      VarTemplateD = VT;
----------------
The purpose of the renaming is less than obvious here.

Can we rename VarTemplateDecl to OuterDecl or so? (And change the type to Decl)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89098