llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Michael Spencer (Bigcheese) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/83641.diff 4 Files Affected: - (modified) clang/lib/Frontend/CompilerInstance.cpp (+20-8) - (removed) clang/test/Modules/Inputs/config.h (-7) - (modified) clang/test/Modules/Inputs/module.modulemap (-5) - (modified) clang/test/Modules/config_macros.m (+42-3) ``````````diff diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 444ffff3073775..14fd140f03cc36 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 (unsigned I = 0, N = TopModule->ConfigMacros.size(); I != N; ++I) { + checkConfigMacro(PP, TopModule->ConfigMacros[I], M, ImportLoc); + } +} + /// Write a new timestamp file with the given path. static void writeTimestampFile(StringRef TimestampFile) { std::error_code EC; @@ -1829,6 +1837,12 @@ 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. + checkConfigMacros(getPreprocessor(), M, ImportLoc); + // Select the source and filename for loading the named module. std::string ModuleFilename; ModuleSource Source = @@ -2006,6 +2020,11 @@ 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( @@ -2146,18 +2165,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..65dd2a89a05c32 100644 --- a/clang/test/Modules/config_macros.m +++ b/clang/test/Modules/config_macros.m @@ -1,3 +1,39 @@ +// RUN: rm -rf %t +// RUN: split-file %s %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 %t/module.modulemap +// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %t -DWANT_FOO=1 %t/config.m -verify +// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %t -DTEST_ERROR %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 +58,10 @@ #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 +#ifdef TEST_ERROR +#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';}} +#endif `````````` </details> https://github.com/llvm/llvm-project/pull/83641 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
