[PATCH] D144074: [clangd] Hide inlay hints when using a macro as a calling argument that with a param comment

[Younan Zhang via Phabricator via cfe-commits]

zyounan created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan added reviewers: kadircet, nridge.
zyounan added a comment.
zyounan retitled this revision from "[clangd] Hide extra inlay hints for macro as argument" to "[clangd] Hide inlay hints when using a macro as a calling argument that with a param comment".
zyounan published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Note that we still have some issues on inlay hints with macro expansion even after D133982 <https://reviews.llvm.org/D133982>.

  void func(int a, int b)
  #define CALL(a, b) func(int(a), b)
  
  CALL(3, 4)  // CALL(3, b: 4)

This is because `spelledForExpanded` doesn't return counterpart spelling ( `int(3)` -> `3` ). (I guess it is due to patch D134618 <https://reviews.llvm.org/D134618>.)

  void func(int a, int b)
  void work(int a, int &b)
  #define CALL(a, b) { func(int(a), b); work(a, b); }
  
  int z = 42;
  CALL(3, z)  // CALL(a: 3, &b:b: 4)

This is because call expression handling logic (i.e. `InlayHintVisitor::processCall`) doesn't know the macro expansion context of expression, resulting producing two inlay hints at the same spelled position.



================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:521
+    auto Decomposed = SM.getDecomposedExpansionLoc(ExprStartLoc);
     if (Decomposed.first != MainFileID)
       return false;
----------------
stupid question: I was suspicious with this check that when would `Decomposed.first` not being the same as MainFileID? Should the "top-caller" of the macro always be in main file? I didn't find a case that differs, except when `getDecomposedLoc` provides wrong FileID.


We don't want to produce inlay hints for arguments for which
user has left param name comments. But we're not decomposing
location of the parameter correctly at the moment because the
location we've passed into `SM.getDecomposedLoc` is not always
FileID.

Fixes clangd/clangd#1495


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144074

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1050,9 +1050,15 @@
     void bar() {
       foo(/*param*/42);
       foo( /* param = */ 42);
+#define X 42
+#define Y X
+#define Z(...) Y
+      foo(/*param=*/Z(a));
+      foo($macro[[Z(a)]]);
       foo(/* the answer */$param[[42]]);
     }
   )cpp",
+                       ExpectedHint{"param: ", "macro"},
                        ExpectedHint{"param: ", "param"});
 }
 
Index: clang-tools-extra/clangd/InlayHints.cpp
===================================================================
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -517,7 +517,7 @@
   bool isPrecededByParamNameComment(const Expr *E, StringRef ParamName) {
     auto &SM = AST.getSourceManager();
     auto ExprStartLoc = SM.getTopMacroCallerLoc(E->getBeginLoc());
-    auto Decomposed = SM.getDecomposedLoc(ExprStartLoc);
+    auto Decomposed = SM.getDecomposedExpansionLoc(ExprStartLoc);
     if (Decomposed.first != MainFileID)
       return false;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144074.497572.patch
Type: text/x-patch
Size: 1219 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230215/45e702ee/attachment.bin>