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