vsapsai added a comment.
Noticed that the warning and the fix-it might not work well with pragmas
suppressing diagnostic and with header guards. But it's not a regression and I
don't think it is worth improving these use cases preemptively.
================
Comment at: clang/lib/Lex/PPLexerChange.cpp:266
void Preprocessor::diagnoseMissingHeaderInUmbrellaDir(const Module &Mod) {
- assert(Mod.getUmbrellaHeader() && "Module must use umbrella header");
- SourceLocation StartLoc =
- SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
- if (getDiagnostics().isIgnored(diag::warn_uncovered_module_header, StartLoc))
+ const auto &UmbrellaHeader = Mod.getUmbrellaHeader();
+ assert(UmbrellaHeader.Entry && "Module must use umbrella header");
----------------
I was going to suggest to spell out the type explicitly instead of `auto`
because I wasn't able to guess the type. But when I've checked,
`Module::Header` didn't look particularly better. I'll leave it up to you to
decide which one is more readable, for wider context there was a discussion
[RFC: Modernizing our use of
auto](https://groups.google.com/forum/#!topic/llvm-dev/GSyITfY1BLg).
================
Comment at: clang/lib/Lex/PPLexerChange.cpp:269
+ const FileID &File = SourceMgr.translateFile(UmbrellaHeader.Entry);
+ SourceLocation EndLoc = SourceMgr.getLocForEndOfFile(File);
+ if (getDiagnostics().isIgnored(diag::warn_uncovered_module_header, EndLoc))
----------------
Can we rename `EndLoc` to better convey its purpose, not how it was computed? I
was thinking about something like `SuggestedHeadersLoc` or `ExpectedHeadersLoc`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82118/new/
https://reviews.llvm.org/D82118
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
