[PATCH] D116387: [CodeCompletion][clangd] Clean __uglified parameter names in completion & hover

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 24 04:22:51 PST 2022


hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:65
   Base.SuppressTemplateArgsInCXXConstructors = true;
+  Base.CleanUglifiedParameters = true;
   return Base;
----------------
IIUC, the code looks like we change the hover behavior as well, but I think our plan is to not change hover. 


================
Comment at: clang/lib/AST/DeclPrinter.cpp:889
+  printDeclType(T, (isa<ParmVarDecl>(D) && Policy.CleanUglifiedParameters &&
+                    D->getIdentifier())
+                       ? D->getIdentifier()->deuglifiedName()
----------------
nit: `D->getIdentifier()` seems redundant -- `D->getName()` has an `isIdentifier()` assertion, we should expect that we can always get identifier from D here. 


================
Comment at: clang/lib/AST/DeclPrinter.cpp:1138
+    if (TTP->getDeclName()) {
+      if (Policy.CleanUglifiedParameters && isa<TemplateTemplateParmDecl>(D) &&
+          TTP->getIdentifier())
----------------
`isa<TemplateTemplateParmDecl>(D)` is redundant as line 1128 has checked it.


================
Comment at: clang/lib/Basic/IdentifierTable.cpp:312
 
+StringRef IdentifierInfo::deuglifiedName() const {
+  StringRef Name = getName();
----------------
Not objecting the current approach -- an alternative is to incline this into `getName` by adding a parameter `Deuglify` to control the behavior.

Not sure it is much better, but it seem to save some boilerplate code like `Policy.CleanUglifiedParameters ? II->deuglifiedName() : II->getName()`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116387



More information about the cfe-commits mailing list