From f2f21c5752e9b1283576d0278cb5018db36f1946 Mon Sep 17 00:00:00 2001 From: Orkun Tokdemir Date: Thu, 25 May 2023 13:35:17 +0200 Subject: Improve Const Correctness --- Source/cmQtAutoGenInitializer.cxx | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Source/cmQtAutoGenInitializer.cxx b/Source/cmQtAutoGenInitializer.cxx index 76bb0cd..bac3f5b 100644 --- a/Source/cmQtAutoGenInitializer.cxx +++ b/Source/cmQtAutoGenInitializer.cxx @@ -344,7 +344,7 @@ bool cmQtAutoGenInitializer::InitCustomTargets() // Verbosity { - std::string def = + std::string const def = this->Makefile->GetSafeDefinition("CMAKE_AUTOGEN_VERBOSE"); if (!def.empty()) { unsigned long iVerb = 0; @@ -546,7 +546,7 @@ bool cmQtAutoGenInitializer::InitCustomTargets() this->Moc.MacroNames.erase(cmRemoveDuplicates(this->Moc.MacroNames), this->Moc.MacroNames.end()); { - cmList filterList = { this->GenTarget->GetSafeProperty( + cmList const filterList = { this->GenTarget->GetSafeProperty( "AUTOMOC_DEPEND_FILTERS") }; if ((filterList.size() % 2) != 0) { cmSystemTools::Error( @@ -650,7 +650,7 @@ bool cmQtAutoGenInitializer::InitMoc() // Moc includes { - SearchPathSanitizer sanitizer(this->Makefile); + SearchPathSanitizer const sanitizer(this->Makefile); auto getDirs = [this, &sanitizer](std::string const& cfg) -> std::vector { // Get the include dirs for this target, without stripping the implicit @@ -1037,7 +1037,7 @@ bool cmQtAutoGenInitializer::InitScanFiles() property = "SKIP_AUTOUIC"; } std::string files; - for (MUFile* muf : this->AutogenTarget.FilesGenerated) { + for (MUFile const* muf : this->AutogenTarget.FilesGenerated) { files += cmStrCat(" ", Quoted(muf->FullPath), '\n'); } this->Makefile->IssueMessage( @@ -1068,7 +1068,7 @@ bool cmQtAutoGenInitializer::InitScanFiles() property = "SKIP_AUTOUIC"; } std::string files; - for (cmSourceFile* sf : this->AutogenTarget.CMP0100HeadersWarn) { + for (cmSourceFile const* sf : this->AutogenTarget.CMP0100HeadersWarn) { files += cmStrCat(" ", Quoted(sf->GetFullPath()), '\n'); } this->Makefile->IssueMessage( @@ -1089,7 +1089,7 @@ bool cmQtAutoGenInitializer::InitScanFiles() if (!this->Rcc.Qrcs.empty()) { const bool modernQt = (this->QtVersion.Major >= 5); // Target rcc options - cmList optionsTarget{ this->GenTarget->GetSafeProperty( + cmList const optionsTarget{ this->GenTarget->GetSafeProperty( kw.AUTORCC_OPTIONS) }; // Check if file name is unique @@ -1213,7 +1213,7 @@ bool cmQtAutoGenInitializer::InitAutogenTarget() // instead of fiddling with the include directories std::vector configs; this->GlobalGen->GetQtAutoGenConfigs(configs); - bool stdPipesUTF8 = true; + bool constexpr stdPipesUTF8 = true; cmCustomCommandLines commandLines; for (auto const& config : configs) { commandLines.push_back(cmMakeCommandLine( @@ -1244,7 +1244,7 @@ bool cmQtAutoGenInitializer::InitAutogenTarget() // Create the autogen target/command if (usePRE_BUILD) { // Add additional autogen target dependencies to origin target - for (cmTarget* depTarget : this->AutogenTarget.DependTargets) { + for (cmTarget const* depTarget : this->AutogenTarget.DependTargets) { this->GenTarget->Target->AddUtility(depTarget->GetName(), false, this->Makefile); } @@ -1747,11 +1747,11 @@ bool cmQtAutoGenInitializer::SetupWriteAutogenInfo() info.SetConfig("MOC_COMPILATION_FILE", this->Moc.CompilationFile); info.SetConfig("MOC_PREDEFS_FILE", this->Moc.PredefsFile); - cmStandardLevelResolver resolver{ this->Makefile }; - auto CompileOptionFlag = + cmStandardLevelResolver const resolver{ this->Makefile }; + auto const CompileOptionFlag = resolver.GetCompileOptionDef(this->GenTarget, "CXX", ""); - auto CompileOptionValue = + auto const CompileOptionValue = this->GenTarget->Makefile->GetSafeDefinition(CompileOptionFlag); if (!CompileOptionValue.empty()) { @@ -1995,13 +1995,13 @@ static cmQtAutoGen::IntegerVersion GetMocVersion( return parseMocVersion(capturedStdOut); } -static std::string FindMocExecutableFromMocTarget(cmMakefile* makefile, +static std::string FindMocExecutableFromMocTarget(cmMakefile const* makefile, unsigned int qtMajorVersion) { std::string result; const std::string mocTargetName = "Qt" + std::to_string(qtMajorVersion) + "::moc"; - cmTarget* mocTarget = makefile->FindTargetToUse(mocTargetName); + cmTarget const* mocTarget = makefile->FindTargetToUse(mocTargetName); if (mocTarget) { result = mocTarget->GetSafeProperty("IMPORTED_LOCATION"); } -- cgit v0.12 From 3bd605f3d06b8ac39f517b5a105c5254ca3ec12b Mon Sep 17 00:00:00 2001 From: Orkun Tokdemir Date: Thu, 25 May 2023 17:33:32 +0200 Subject: Autogen: Optimize cmake_autogen execution for CROSS_CONFIG usage The redundant `cmake_autogen` process execution was optimized for non-`CROSS_CONFIGS` usage. It was executed three times for each config although only one of them is needed. --- Source/cmQtAutoGenInitializer.cxx | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/Source/cmQtAutoGenInitializer.cxx b/Source/cmQtAutoGenInitializer.cxx index bac3f5b..62d7ffb 100644 --- a/Source/cmQtAutoGenInitializer.cxx +++ b/Source/cmQtAutoGenInitializer.cxx @@ -1168,7 +1168,8 @@ bool cmQtAutoGenInitializer::InitAutogenTarget() if (this->Moc.Enabled) { this->AddGeneratedSource(this->Moc.CompilationFile, this->Moc, true); if (useNinjaDepfile) { - if (this->MultiConfig) { + if (this->MultiConfig && + !this->Makefile->GetSafeDefinition("CMAKE_CROSS_CONFIGS").empty()) { // Make all mocs_compilation_.cpp files byproducts of the // ${target}_autogen/timestamp custom command. // We cannot just use Moc.CompilationFileGenex here, because that @@ -1215,10 +1216,23 @@ bool cmQtAutoGenInitializer::InitAutogenTarget() this->GlobalGen->GetQtAutoGenConfigs(configs); bool constexpr stdPipesUTF8 = true; cmCustomCommandLines commandLines; - for (auto const& config : configs) { + if (this->Makefile->GetSafeDefinition("CMAKE_CROSS_CONFIGS").empty()) { + std::string autugenInfoFileconfig; + if (this->MultiConfig) { + autugenInfoFileconfig = "$"; + } else { + autugenInfoFileconfig = configs[0]; + } commandLines.push_back(cmMakeCommandLine( { cmSystemTools::GetCMakeCommand(), "-E", "cmake_autogen", - this->AutogenTarget.InfoFile, config })); + this->AutogenTarget.InfoFile, autugenInfoFileconfig })); + + } else { + for (auto const& config : configs) { + commandLines.push_back(cmMakeCommandLine( + { cmSystemTools::GetCMakeCommand(), "-E", "cmake_autogen", + this->AutogenTarget.InfoFile, config })); + } } // Use PRE_BUILD on demand -- cgit v0.12 From 2bb3d9b6448efd2aab4db3e33a16b22a1b1b097f Mon Sep 17 00:00:00 2001 From: Orkun Tokdemir Date: Thu, 25 May 2023 17:34:35 +0200 Subject: Autogen: Fix multi-config generated file issue The default config was an empty string when a `multi-config` generator is used. An if check was added for those situations. If a source file has a specific config configuration, it is used with `$` in the `multi-config` generator usage. Fixes: #24848 --- Source/cmQtAutoGenInitializer.cxx | 49 +++++++++++++++++++++------ Tests/RunCMake/Autogen/MocGeneratedFile.cmake | 15 ++++++++ Tests/RunCMake/Autogen/RunCMakeTest.cmake | 19 +++++++++++ 3 files changed, 73 insertions(+), 10 deletions(-) create mode 100644 Tests/RunCMake/Autogen/MocGeneratedFile.cmake diff --git a/Source/cmQtAutoGenInitializer.cxx b/Source/cmQtAutoGenInitializer.cxx index 62d7ffb..2c48e78 100644 --- a/Source/cmQtAutoGenInitializer.cxx +++ b/Source/cmQtAutoGenInitializer.cxx @@ -662,8 +662,6 @@ bool cmQtAutoGenInitializer::InitMoc() return sanitizer(dirs); }; - // Default configuration include directories - this->Moc.Includes.Default = getDirs(this->ConfigDefault); // Other configuration settings if (this->MultiConfig) { for (std::string const& cfg : this->ConfigsList) { @@ -673,6 +671,9 @@ bool cmQtAutoGenInitializer::InitMoc() } this->Moc.Includes.Config[cfg] = std::move(dirs); } + } else { + // Default configuration include directories + this->Moc.Includes.Default = getDirs(this->ConfigDefault); } } @@ -690,8 +691,6 @@ bool cmQtAutoGenInitializer::InitMoc() return defines; }; - // Default configuration defines - this->Moc.Defines.Default = getDefs(this->ConfigDefault); // Other configuration defines if (this->MultiConfig) { for (std::string const& cfg : this->ConfigsList) { @@ -701,6 +700,9 @@ bool cmQtAutoGenInitializer::InitMoc() } this->Moc.Defines.Config[cfg] = std::move(defines); } + } else { + // Default configuration defines + this->Moc.Defines.Default = getDefs(this->ConfigDefault); } } @@ -1024,8 +1026,24 @@ bool cmQtAutoGenInitializer::InitScanFiles() if (this->MocOrUicEnabled() && !this->AutogenTarget.FilesGenerated.empty()) { if (this->CMP0071Accept) { // Let the autogen target depend on the GENERATED files - for (MUFile* muf : this->AutogenTarget.FilesGenerated) { - this->AutogenTarget.DependFiles.insert(muf->FullPath); + if (this->MultiConfig && + this->Makefile->GetSafeDefinition("CMAKE_CROSS_CONFIGS").empty()) { + for (MUFile const* muf : this->AutogenTarget.FilesGenerated) { + if (muf->Configs.empty()) { + this->AutogenTarget.DependFiles.insert(muf->FullPath); + } else { + for (size_t ci : muf->Configs) { + std::string const& config = this->ConfigsList[ci]; + std::string const& pathWithConfig = + cmStrCat("$<$:", muf->FullPath, '>'); + this->AutogenTarget.DependFiles.insert(pathWithConfig); + } + } + } + } else { + for (MUFile const* muf : this->AutogenTarget.FilesGenerated) { + this->AutogenTarget.DependFiles.insert(muf->FullPath); + } } } else if (this->CMP0071Warn) { cm::string_view property; @@ -1738,10 +1756,21 @@ bool cmQtAutoGenInitializer::SetupWriteAutogenInfo() this->GenTarget, "AUTOMOC_MACRO_NAMES", nullptr, nullptr); EvaluatedTargetPropertyEntries InterfaceAutoMocMacroNamesEntries; - AddInterfaceEntries(this->GenTarget, this->ConfigDefault, - "INTERFACE_AUTOMOC_MACRO_NAMES", "CXX", &dagChecker, - InterfaceAutoMocMacroNamesEntries, - IncludeRuntimeInterface::Yes); + if (this->MultiConfig) { + for (auto const& cfg : this->ConfigsList) { + if (!cfg.empty()) { + AddInterfaceEntries(this->GenTarget, cfg, + "INTERFACE_AUTOMOC_MACRO_NAMES", "CXX", + &dagChecker, InterfaceAutoMocMacroNamesEntries, + IncludeRuntimeInterface::Yes); + } + } + } else { + AddInterfaceEntries(this->GenTarget, this->ConfigDefault, + "INTERFACE_AUTOMOC_MACRO_NAMES", "CXX", &dagChecker, + InterfaceAutoMocMacroNamesEntries, + IncludeRuntimeInterface::Yes); + } for (auto const& entry : InterfaceAutoMocMacroNamesEntries.Entries) { this->Moc.MacroNames.insert(this->Moc.MacroNames.end(), diff --git a/Tests/RunCMake/Autogen/MocGeneratedFile.cmake b/Tests/RunCMake/Autogen/MocGeneratedFile.cmake new file mode 100644 index 0000000..7bb55e9 --- /dev/null +++ b/Tests/RunCMake/Autogen/MocGeneratedFile.cmake @@ -0,0 +1,15 @@ +enable_language(CXX) + +find_package(Qt${with_qt_version} REQUIRED COMPONENTS Core) + +set(CMAKE_AUTOMOC ON) + +set(GEN_SRC "class_$.cpp") +add_custom_command( + OUTPUT "${GEN_SRC}" + COMMAND ${CMAKE_COMMAND} -E echo "// cpp src" > "${GEN_SRC}" + VERBATIM +) + +add_library(libgen STATIC ${GEN_SRC}) +target_link_libraries(libgen Qt${with_qt_version}::Core) diff --git a/Tests/RunCMake/Autogen/RunCMakeTest.cmake b/Tests/RunCMake/Autogen/RunCMakeTest.cmake index 97b64ed..4fe9406 100644 --- a/Tests/RunCMake/Autogen/RunCMakeTest.cmake +++ b/Tests/RunCMake/Autogen/RunCMakeTest.cmake @@ -103,4 +103,23 @@ if (DEFINED with_qt_version) endblock() endif() endif() + + if(RunCMake_GENERATOR_IS_MULTI_CONFIG AND NOT RunCMake_GENERATOR MATCHES "Xcode") + block() + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/MocGeneratedFile-build) + run_cmake(MocGeneratedFile) + set(RunCMake_TEST_NO_CLEAN 1) + run_cmake_command(MocGeneratedFile-build ${CMAKE_COMMAND} --build . --config Debug --verbose) + endblock() + if(RunCMake_GENERATOR MATCHES "Ninja Multi-Config") + block() + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/MocGeneratedFile-cross-config-build) + list(APPEND RunCMake_TEST_OPTIONS -DCMAKE_CROSS_CONFIGS=all) + run_cmake(MocGeneratedFile) + set(RunCMake_TEST_NO_CLEAN 1) + run_cmake_command(MocGeneratedFile-cross-config-build ${CMAKE_COMMAND} --build . --config Release --target libgen:Debug) + run_cmake_command(MocGeneratedFile-cross-config-build ${CMAKE_COMMAND} --build . --config Debug --target libgen:Release) + endblock() + endif() + endif() endif () -- cgit v0.12