Author: Michael Spencer Date: 2024-03-05T10:15:21-08:00 New Revision: ee044d5e651787c5d73b37b2cbb7ca6444bb0502
URL: https://github.com/llvm/llvm-project/commit/ee044d5e651787c5d73b37b2cbb7ca6444bb0502 DIFF: https://github.com/llvm/llvm-project/commit/ee044d5e651787c5d73b37b2cbb7ca6444bb0502.diff LOG: [clang] Diagnose config_macros before building modules (#83641) Before this patch, if a module fails to build because of a missing config_macro, the user will never see the config macro warning. This patch diagnoses this before building, and each subsequent time a module is imported. rdar://123921931 Added: Modified: clang/lib/Frontend/CompilerInstance.cpp clang/test/Modules/Inputs/module.modulemap clang/test/Modules/config_macros.m Removed: clang/test/Modules/Inputs/config.h ################################################################################ diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 444ffff3073775..ec4e68209d657d 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1591,6 +1591,14 @@ static void checkConfigMacro(Preprocessor &PP, StringRef ConfigMacro, } } +static void checkConfigMacros(Preprocessor &PP, Module *M, + SourceLocation ImportLoc) { + clang::Module *TopModule = M->getTopLevelModule(); + for (const StringRef ConMacro : TopModule->ConfigMacros) { + checkConfigMacro(PP, ConMacro, M, ImportLoc); + } +} + /// Write a new timestamp file with the given path. static void writeTimestampFile(StringRef TimestampFile) { std::error_code EC; @@ -1829,6 +1837,13 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST( Module *M = HS.lookupModule(ModuleName, ImportLoc, true, !IsInclusionDirective); + // Check for any configuration macros that have changed. This is done + // immediately before potentially building a module in case this module + // depends on having one of its configuration macros defined to successfully + // build. If this is not done the user will never see the warning. + if (M) + checkConfigMacros(getPreprocessor(), M, ImportLoc); + // Select the source and filename for loading the named module. std::string ModuleFilename; ModuleSource Source = @@ -2006,12 +2021,23 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) { // Use the cached result, which may be nullptr. Module = *MaybeModule; + // Config macros are already checked before building a module, but they need + // to be checked at each import location in case any of the config macros + // have a new value at the current `ImportLoc`. + if (Module) + checkConfigMacros(getPreprocessor(), Module, ImportLoc); } else if (ModuleName == getLangOpts().CurrentModule) { // This is the module we're building. Module = PP->getHeaderSearchInfo().lookupModule( ModuleName, ImportLoc, /*AllowSearch*/ true, /*AllowExtraModuleMapSearch*/ !IsInclusionDirective); + // Config macros do not need to be checked here for two reasons. + // * This will always be textual inclusion, and thus the config macros + // actually do impact the content of the header. + // * `Preprocessor::HandleHeaderIncludeOrImport` will never call this + // function as the `#include` or `#import` is textual. + MM.cacheModuleLoad(*Path[0].first, Module); } else { ModuleLoadResult Result = findOrCompileModuleAndReadAST( @@ -2146,18 +2172,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, TheASTReader->makeModuleVisible(Module, Visibility, ImportLoc); } - // Check for any configuration macros that have changed. - clang::Module *TopModule = Module->getTopLevelModule(); - for (unsigned I = 0, N = TopModule->ConfigMacros.size(); I != N; ++I) { - checkConfigMacro(getPreprocessor(), TopModule->ConfigMacros[I], - Module, ImportLoc); - } - // Resolve any remaining module using export_as for this one. getPreprocessor() .getHeaderSearchInfo() .getModuleMap() - .resolveLinkAsDependencies(TopModule); + .resolveLinkAsDependencies(Module->getTopLevelModule()); LastModuleImportLoc = ImportLoc; LastModuleImportResult = ModuleLoadResult(Module); diff --git a/clang/test/Modules/Inputs/config.h b/clang/test/Modules/Inputs/config.h deleted file mode 100644 index 4c124b0bf82b7c..00000000000000 --- a/clang/test/Modules/Inputs/config.h +++ /dev/null @@ -1,7 +0,0 @@ -#ifdef WANT_FOO -int* foo(void); -#endif - -#ifdef WANT_BAR -char *bar(void); -#endif diff --git a/clang/test/Modules/Inputs/module.modulemap b/clang/test/Modules/Inputs/module.modulemap index e7cb4b27bc08b9..47f6c5c1010d7d 100644 --- a/clang/test/Modules/Inputs/module.modulemap +++ b/clang/test/Modules/Inputs/module.modulemap @@ -260,11 +260,6 @@ module cxx_decls_merged { header "cxx-decls-merged.h" } -module config { - header "config.h" - config_macros [exhaustive] WANT_FOO, WANT_BAR -} - module diag_flags { header "diag_flags.h" } diff --git a/clang/test/Modules/config_macros.m b/clang/test/Modules/config_macros.m index 15e2c16606ba29..adb2a8f6e5ce34 100644 --- a/clang/test/Modules/config_macros.m +++ b/clang/test/Modules/config_macros.m @@ -1,3 +1,50 @@ +// This test verifies that config macro warnings are emitted when it looks like +// the user expected a `#define` to impact the import of a module. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +// Prebuild the `config` module so it's in the module cache. +// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -x objective-c -fmodules-cache-path=%t -DWANT_FOO=1 -emit-module -fmodule-name=config %t/module.modulemap + +// Verify that each time the `config` module is imported the current macro state +// is checked. +// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %t -DWANT_FOO=1 %t/config.m -verify + +// Verify that warnings are emitted before building a module in case the command +// line macro state causes the module to fail to build. +// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %t %t/config_error.m -verify + +//--- module.modulemap + +module config { + header "config.h" + config_macros [exhaustive] WANT_FOO, WANT_BAR +} + +module config_error { + header "config_error.h" + config_macros SOME_VALUE +} + +//--- config.h + +#ifdef WANT_FOO +int* foo(void); +#endif + +#ifdef WANT_BAR +char *bar(void); +#endif + +//--- config_error.h + +struct my_thing { + char buf[SOME_VALUE]; +}; + +//--- config.m + @import config; int *test_foo(void) { @@ -22,7 +69,8 @@ #define WANT_BAR 1 // expected-note{{macro was defined here}} @import config; // expected-warning{{definition of configuration macro 'WANT_BAR' has no effect on the import of 'config'; pass '-DWANT_BAR=...' on the command line to configure the module}} -// RUN: rm -rf %t -// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -x objective-c -fmodules-cache-path=%t -DWANT_FOO=1 -emit-module -fmodule-name=config %S/Inputs/module.modulemap -// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs -DWANT_FOO=1 %s -verify +//--- config_error.m +#define SOME_VALUE 5 // expected-note{{macro was defined here}} +@import config_error; // expected-error{{could not build module}} \ + // expected-warning{{definition of configuration macro 'SOME_VALUE' has no effect on the import of 'config_error';}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
