kadircet marked an inline comment as done.
kadircet added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:43
+ // This results in surprising behavior from users point of view (we
+ // generate a usage of stdio.h, in places unrelated to standard library).
+ // FIXME: Also eliminate the false positives by treating declarations
----------------
sammccall wrote:
> Comment nit: I'm having trouble imagining cases that are actually*unrelated*
> to the stdlib.
>
> If `stderr` is an impl-defined macro, then the only way to use the name to
> refer to something else is if it's not defined (inside ifndef stderr, or if
> you can be sure your TU doesn't include stdio first). Seems implausible...
>
> That said I feel like we've had this conversation before and I've just
> forgotten the details.
> If stderr is an impl-defined macro, then the only way to use the name to
> refer to something else is if it's not defined
That's absolutely right. The issue here is macro expansion triggers independent
of the context, e.g.
```
#include <stdio.h>
namespace ns { void stderr(); }
void foo() { ns::stderr(); }
```
here we have a (well two) reference to `stderr` macro from stdandard library,
which is not user's intent, but rather a quirk of the language.
================
Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:44
+ // generate a usage of stdio.h, in places unrelated to standard library).
+ // FIXME: Also eliminate the false positives by treating declarations
+ // resulting from these expansions as used.
----------------
sammccall wrote:
> Nit: "false positives" is a little unclear: positives for this function are
> negatives for walkAST and could be either for diagnostics.
>
> Also, I think you mean *references* rather than declarations?
>
this should've been "false negatives" from the "usedness perspective", we
basically drop the reference to `ns::stderr` in above example, because the name
is not spelled in the main file (but rather spelled inside a macro body).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157905/new/
https://reviews.llvm.org/D157905
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
