llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang <details> <summary>Changes</summary> The `Module` parameter to `checkModuleIsAvailable` is currently passed by pointer to non-const. However, it requires only const access and it cannot be null. Change this to be a reference to const instead. This then makes it obvious that it is an input-only parameter, so move it to be before the in-out parameter for diagnostics. --- Full diff: https://github.com/llvm/llvm-project/pull/67902.diff 5 Files Affected: - (modified) clang/include/clang/Lex/Preprocessor.h (+1-1) - (modified) clang/lib/Frontend/CompilerInstance.cpp (+1-1) - (modified) clang/lib/Frontend/FrontendAction.cpp (+2-2) - (modified) clang/lib/Lex/PPDirectives.cpp (+10-8) - (modified) clang/lib/Lex/Pragma.cpp (+1-1) ``````````diff diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index e88164f196c1f7c..b9423640d62f741 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -2697,7 +2697,7 @@ class Preprocessor { /// \c false if the module appears to be usable. static bool checkModuleIsAvailable(const LangOptions &LangOpts, const TargetInfo &TargetInfo, - DiagnosticsEngine &Diags, Module *M); + const Module &M, DiagnosticsEngine &Diags); // Module inclusion testing. /// Find the module that owns the source or header file that diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index d18371f21a9d86e..d749195585eca5b 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -2116,7 +2116,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, // Check whether this module is available. if (Preprocessor::checkModuleIsAvailable(getLangOpts(), getTarget(), - getDiagnostics(), Module)) { + *Module, getDiagnostics())) { getDiagnostics().Report(ImportLoc, diag::note_module_import_here) << SourceRange(Path.front().second, Path.back().second); LastModuleImportLoc = ImportLoc; diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 60eac0c6d353048..eb8a96627bb7076 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -510,8 +510,8 @@ static Module *prepareToBuildModule(CompilerInstance &CI, } // Check whether we can build this module at all. - if (Preprocessor::checkModuleIsAvailable(CI.getLangOpts(), CI.getTarget(), - CI.getDiagnostics(), M)) + if (Preprocessor::checkModuleIsAvailable(CI.getLangOpts(), CI.getTarget(), *M, + CI.getDiagnostics())) return nullptr; // Inform the preprocessor that includes from within the input buffer should diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 7899bfa1c4f5842..e3065c17dc70b43 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -1896,26 +1896,27 @@ static bool trySimplifyPath(SmallVectorImpl<StringRef> &Components, bool Preprocessor::checkModuleIsAvailable(const LangOptions &LangOpts, const TargetInfo &TargetInfo, - DiagnosticsEngine &Diags, Module *M) { + const Module &M, + DiagnosticsEngine &Diags) { Module::Requirement Requirement; Module::UnresolvedHeaderDirective MissingHeader; Module *ShadowingModule = nullptr; - if (M->isAvailable(LangOpts, TargetInfo, Requirement, MissingHeader, - ShadowingModule)) + if (M.isAvailable(LangOpts, TargetInfo, Requirement, MissingHeader, + ShadowingModule)) return false; if (MissingHeader.FileNameLoc.isValid()) { Diags.Report(MissingHeader.FileNameLoc, diag::err_module_header_missing) << MissingHeader.IsUmbrella << MissingHeader.FileName; } else if (ShadowingModule) { - Diags.Report(M->DefinitionLoc, diag::err_module_shadowed) << M->Name; + Diags.Report(M.DefinitionLoc, diag::err_module_shadowed) << M.Name; Diags.Report(ShadowingModule->DefinitionLoc, diag::note_previous_definition); } else { // FIXME: Track the location at which the requirement was specified, and // use it here. - Diags.Report(M->DefinitionLoc, diag::err_module_unavailable) - << M->getFullModuleName() << Requirement.second << Requirement.first; + Diags.Report(M.DefinitionLoc, diag::err_module_unavailable) + << M.getFullModuleName() << Requirement.second << Requirement.first; } return true; } @@ -2260,8 +2261,9 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // unavailable, diagnose the situation and bail out. // FIXME: Remove this; loadModule does the same check (but produces // slightly worse diagnostics). - if (checkModuleIsAvailable(getLangOpts(), getTargetInfo(), getDiagnostics(), - SuggestedModule.getModule())) { + if (checkModuleIsAvailable(getLangOpts(), getTargetInfo(), + *SuggestedModule.getModule(), + getDiagnostics())) { Diag(FilenameTok.getLocation(), diag::note_implicit_top_level_module_import_here) << SuggestedModule.getModule()->getTopLevelModuleName(); diff --git a/clang/lib/Lex/Pragma.cpp b/clang/lib/Lex/Pragma.cpp index 58da4410dee64a4..35ab42cb6b5ef85 100644 --- a/clang/lib/Lex/Pragma.cpp +++ b/clang/lib/Lex/Pragma.cpp @@ -1769,7 +1769,7 @@ struct PragmaModuleBeginHandler : public PragmaHandler { // If the module isn't available, it doesn't make sense to enter it. if (Preprocessor::checkModuleIsAvailable( - PP.getLangOpts(), PP.getTargetInfo(), PP.getDiagnostics(), M)) { + PP.getLangOpts(), PP.getTargetInfo(), *M, PP.getDiagnostics())) { PP.Diag(BeginLoc, diag::note_pp_module_begin_here) << M->getTopLevelModuleName(); return; `````````` </details> https://github.com/llvm/llvm-project/pull/67902 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits