PiotrZSL marked 4 inline comments as done.
PiotrZSL added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp:134-135
+ do {
+ if (CurrentIt->Name.empty())
+ continue;
+
----------------
njames93 wrote:
> Is this a case that can ever come up?
> If it does come up, surely any notes emitted after that would be garbage, in
> which case wouldn't it make more sense to break rather than continue?
I will remove this...
================
Comment at: clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp:146
+ if (Location.isInvalid())
+ continue;
+
----------------
njames93 wrote:
> Again whats the rationale for using continue over break here?
Idea is to provide any information, even if it could be missing something.
================
Comment at:
clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp:148-150
+ Check.diag(Location, "'%0' included from command line",
+ DiagnosticIDs::Note)
+ << CurrentIt->Name;
----------------
njames93 wrote:
> Is this a case that is expected to come up, if so can there be a test added.
> Likewise if we do have a file that was included from the command line,
> surely we should break after emitting that diagnostic, It's not like the
> command line can be included from another file.
I didn't found any case it could be, added this as an preclusion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144828/new/
https://reviews.llvm.org/D144828
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
