VitaNuo marked an inline comment as done.
VitaNuo added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:237
+ auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin()));
+ if (!FE) {
+ return false;
----------------
kadircet wrote:
> nit: you can drop the braces here. LLVM convention is to not have braces when
> a condition/loop body has only a single statement.
Ah ok, thank you. It's a bit funny, my whole life until now I've been
rigorously taught to never do this ;-)
================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:240
+ }
+ if (Pragma->consume_front(", include ")) {
+ // We always insert using the spelling from the pragma.
----------------
kadircet wrote:
> nit: you can also rewrite this as:
> ```
> StringRef PublicHeader;
> if (Pragma->consume_front(", include ")) {
> PublicHeader = save(Pragma->startswith("<") || Pragma->startswith("\"")
> ? (*Pragma)
> : ("\"" + *Pragma + "\"").str());
> }
> Out->IWYUPublic.insert({FE->getLastRef().getUniqueID(), PublicHeader});
> ```
Yeah this is good, thanks.
================
Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:381
+ // IWYU pragma: private
+ class Private {};
+ )cpp";
----------------
kadircet wrote:
> VitaNuo wrote:
> > kadircet wrote:
> > > nit: no need for the declaration of `Private` here (nor in `private.h`).
> > > it's actually introducing a double definition error into TU now.
> > Out of curiosity: what's TU?
> it means `translation unit`, it's the source file provided to the compiler
> after all preprocessor directives are processed.
Thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138678/new/
https://reviews.llvm.org/D138678
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
