kadircet marked 2 inline comments 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:
> 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?
> Sure, but this code is not valid C++ (since stderr is not guaranteed to
> expand to a single identifier).
That's true. Most of the standard library implementations I checked does that,
but I also don't like that reliance, so making it a little bit more generic
(which meant some more plumbing, sorry for the big diff :( ), by making sure
that we apply this to all the macros that expand to themselves.
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
