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
