aaronpuchert added a comment.

In D59402#1515703 <https://reviews.llvm.org/D59402#1515703>, @aaron.ballman 
wrote:

> My only real concern about this is that sometimes the fix-it will be wrong 
> because the issue is a typo in the definition.


You're right about that, there are multiple reasons why one would see the 
warning. But if your program compiles and links fine, then this seems to be the 
only reason left.

- If the function is in a header and should be declared `inline`, you're most 
likely getting linker errors with duplicate symbols.
- If there is a typo, and the definition doesn't match the declaration, there 
should also be a linker error about a missing symbol.

We had a discussion in our team about activating the warning, and a colleague 
of mine remarked "to fix this, you would have to declare every function before 
defining it, even when its only declared & defined in a source file". That's 
when I realized the warning might be a bit misleading, and suggesting to add 
`static` could fix that. It's also how the warning should be fixed in our code 
in almost all cases. (There are a few cases where it fires in a header that is 
only included once.)


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