From 72ab9612a750292a5767861d17616e4e431024a3 Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Mon, 13 Nov 2017 16:34:39 +0100 Subject: [PATCH 1/3] [cxxmodules] Also load STL/libc as a core module Without modules, many STL and libc headers are automatically provided by ROOT via the attached ROOT PCH. This means that we don't need to have autloading or explicit includes for STL or libc headers when running with the PCH attached. This leads to making user code like this working in ROOT: ```C++ // no includes here that provides assert int foo() { assert(false); } ``` However, as the modules don't come with this big PCH, we are now suddenly in the situation where we can't resolve things such as `assert`. We also can't rely on the normal autoloading of ROOT as those declarations were actually never autoloaded, but just provided by the PCH. To simulate this behavior with modules, we automatically load those headers that we expect to have in the ROOT PCH (which are probably the STL and libc headers). --- core/metacling/src/TCling.cxx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 5af6fe7d514eb..d925b92fbf668 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -1101,19 +1101,30 @@ static void LoadCoreModules(cling::Interpreter &interp) clang::HeaderSearch &headerSearch = CI.getPreprocessor().getHeaderSearchInfo(); clang::ModuleMap &moduleMap = headerSearch.getModuleMap(); + // List of core modules we can load, but it's ok if they are missing because + // the system doesn't have these modules. + std::vector optionalCoreModuleNames = {"stl", "libc"}; + // List of core modules we need to load. std::vector neededCoreModuleNames = {"Core", "RIO"}; std::vector missingCoreModuleNames; std::vector coreModules; + // Lookup the optional core modules in the modulemap by name. + for (std::string moduleName : optionalCoreModuleNames) { + clang::Module *module = moduleMap.findModule(moduleName); + // Don't report an error here, the module is optional. + if (module) + coreModules.push_back(module); + } // Lookup the core modules in the modulemap by name. for (std::string moduleName : neededCoreModuleNames) { clang::Module *module = moduleMap.findModule(moduleName); if (module) { coreModules.push_back(module); } else { - // If we can't find a module, we record that to report it later. + // If we can't find a needed module, we record that to report it later. missingCoreModuleNames.push_back(moduleName); } } From 67acc361823c197dc5e243a4475fd106393779a2 Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Tue, 14 Nov 2017 11:40:38 +0100 Subject: [PATCH 2/3] [cxxmodules] #undef I coming from complex.h If we preload the core modules that can contain complex.h, we break a lot of tests that name variables or template arguments I. --- core/metacling/src/TCling.cxx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index d925b92fbf668..f366ce4108f95 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -1163,6 +1163,15 @@ static void LoadCoreModules(cling::Interpreter &interp) for (StringRef H : moduleHeaders) { declarations << "#include \"" << H.str() << "\"\n"; } + + // C99 decided that it's a very good idea to name a macro `I` (the letter I). + // This seems to screw up nearly all the template code out there as `I` is + // common template parameter name and iterator variable name. + // Let's follow the GCC recommendation and undefine `I` in case any of the + // core modules have defined it: + // https://www.gnu.org/software/libc/manual/html_node/Complex-Numbers.html + declarations << "#ifdef I\n #undef I\n #endif\n"; + auto result = interp.declare(declarations.str()); if (result != cling::Interpreter::CompilationResult::kSuccess) { From bb580fe390326fec0f1e01e405c104484e9b70fb Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Tue, 14 Nov 2017 16:59:11 +0100 Subject: [PATCH 3/3] [cxxmodules] Also load TreePlayer to fix TDataFrame tests. It seems automatically loading TreePlayer when TDataFrame is used without the appropriate include never worked. This was previously fixed by just adding TreePlayer (which contains TDF) to the PCH. As always, let's recreate this hack with modules to make restore the old behavior with modules turned on. --- core/metacling/src/TCling.cxx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index f366ce4108f95..2dea465023cfe 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -1106,7 +1106,10 @@ static void LoadCoreModules(cling::Interpreter &interp) std::vector optionalCoreModuleNames = {"stl", "libc"}; // List of core modules we need to load. - std::vector neededCoreModuleNames = {"Core", "RIO"}; + // FIXME: TreePlayer is here because the rootmap loading doesn't work with + // TDataFrame. This should be gone once we no longer rely on rootmap files + // for loading a module. + std::vector neededCoreModuleNames = {"Core", "RIO", "TreePlayer"}; std::vector missingCoreModuleNames; std::vector coreModules;