aaronpuchert added inline comments.
================
Comment at: lib/Sema/SemaDecl.cpp:13345-13346
+            << (FD->getStorageClass() == SC_None
+                    ? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(),
+                                                 "static ")
+                    : FixItHint{});
----------------
aaron.ballman wrote:
> We may not want to produce the fixit if there's a macro involved in the 
> declaration. Consider:
> ```
> #ifdef SOMETHING
> #define FROBBLE static
> #else
> #define FROBBLE
> #endif
> 
> FROBBLE void foo(void);
> ```
> We probably don't want the fixit in the case `SOMETHING` is not defined.
I think that's generally an issue with fix-its, there could always be a macro 
that turns the code into something entirely different. If we look at the other 
fix-it above, we can construct

```
#define VOID
int f(VOID);
int f() { return 0; }
```

Then we get:
```
<stdin>:3:5: warning: no previous prototype for function 'f' 
[-Wmissing-prototypes]
int f() { return 0; }
    ^
<stdin>:2:5: note: this declaration is not a prototype; add 'void' to make it a 
prototype for a zero-parameter function
int f(VOID);
    ^
          void
```

Then the fix-it doesn't even work in the original configuration, because it 
produces `int f(VOIDvoid)`. If we make it work by adding a space, we still have 
the problem that you mentioned: if someone defines `VOID` as `void`, we then 
have `int f(void void)` after applying the fix-it in the original setting.

Trying to make fix-its work with macros is probably a hopeless endeavor.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

Reply via email to