[PATCH] D78338: [clangd] Enable diagnostic fixes within macro argument expansions.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563
+    for (auto &FixIt : FixIts) {
+      // Allow fixits within a single macro-arg expansion to be applied.
+      if (FixIt.RemoveRange.getBegin().isMacroID() &&
----------------
I feel a bit nervous about this (even it is for macro-arg expansion only), as macro is very tricky.

I think we may result in invalid code after applying the fixits in some cases:

1) if the fix is to remove an unused variable (interestingly, clang doesn't provide fixit to remove an unused variable, but for unused lambda capture, it does)

```
#define LA(arg1, arg2) [arg1, arg2] { return arg2;}
void test1(int x, int y) {
  LA(x, y); // after fixit, it would become LA(, y)? or LA(y)
}
```

2) if the fix is to add some characters to the macro argument, e.g. adding a dereference `*`, the semantic of the code after macro expansion maybe changed.

```
void f1(int &a);
void f2(int *a) {
   f1(a); // clang will emits a diagnostic with a fixit adding preceding a `*` to a.
}
```

maybe we should be more conservative? just whitelist some diagnostics? fixing typos seems pretty safe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78338