ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Please take a look the NIT about landing this as two commits.



================
Comment at: clang-tools-extra/clangd/Hover.cpp:581
   HoverInfo HI;
-  const ASTContext &Ctx = D->getASTContext();
+  ASTContext &Ctx = D->getASTContext();
 
----------------
NIT: maybe use `auto&` here?
The name of the type is mentioned directly in the initialization.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:625
   else if (const auto *TN = dyn_cast<TypedefNameDecl>(D))
-    HI.Type = printType(TN->getUnderlyingType(), TN->getASTContext(), PP);
+    HI.Type = printType(TN->getUnderlyingType().getDesugaredType(Ctx), Ctx, 
PP);
   else if (const auto *TAT = dyn_cast<TypeAliasTemplateDecl>(D))
----------------
NIT: could you please land the refactoring with `ASTContext` and this change 
separately?
It was a bit hard to find the relevant change for me, I suspect reading the git 
history might also be problematic in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127832

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

Reply via email to