aaron.ballman added inline comments.
================ Comment at: lib/Sema/SemaDecl.cpp:13345-13346 + << (FD->getStorageClass() == SC_None + ? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(), + "static ") + : FixItHint{}); ---------------- aaronpuchert wrote: > 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. > Trying to make fix-its work with macros is probably a hopeless endeavor. That's what I was getting at -- I think you need to see if there's a macro at the insertion location and not generate a fix-it in that case. The above suffers from the same issue (and can be corrected in a subsequent patch, if desired). 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