sammccall accepted this revision.
sammccall added a comment.
still LG, comments are still confusing me a little
================
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
----------------
kadircet wrote:
> 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.
Sure, but this code is not valid C++ (since `stderr` is not guaranteed to
expand to a single identifier). Is this actually a common/motivating case?
================
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.
----------------
kadircet wrote:
> 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).
And now it's backwards if your perspective is this function itself!
just avoid "false positives"/"false negatives" terminology and describe the
actual effect?
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
