From 9916d4dd44dcb86571ff51a4e4fabb6fe14cec17 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 6 Apr 2022 21:32:40 -0400 Subject: cmTarget: factor out fileset type handling This allows for new fileset types to be added more easily by factoring out the declarative information into a structure. --- Source/cmTarget.cxx | 335 +++++++++++++-------- Tests/RunCMake/target_sources/FileSetImport.cmake | 4 +- .../target_sources/FileSetProperties.cmake | 4 +- 3 files changed, 207 insertions(+), 136 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index a3746ab..446964c 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -165,6 +165,66 @@ cmValue cmTargetPropertyComputer::GetSources(cmTarget const* tgt, return cmValue(srcs); } +namespace { +struct FileSetEntries +{ + FileSetEntries(cm::static_string_view propertyName) + : PropertyName(propertyName) + { + } + + cm::static_string_view const PropertyName; + std::vector> Entries; +}; + +struct FileSetType +{ + FileSetType(cm::static_string_view typeName, + cm::static_string_view defaultDirectoryProperty, + cm::static_string_view defaultPathProperty, + cm::static_string_view directoryPrefix, + cm::static_string_view pathPrefix, + cm::static_string_view typeDescription, + cm::static_string_view defaultDescription, + cm::static_string_view arbitraryDescription, + FileSetEntries selfEntries, FileSetEntries interfaceEntries) + : TypeName(typeName) + , DefaultDirectoryProperty(defaultDirectoryProperty) + , DefaultPathProperty(defaultPathProperty) + , DirectoryPrefix(directoryPrefix) + , PathPrefix(pathPrefix) + , TypeDescription(typeDescription) + , DefaultDescription(defaultDescription) + , ArbitraryDescription(arbitraryDescription) + , SelfEntries(std::move(selfEntries)) + , InterfaceEntries(std::move(interfaceEntries)) + { + } + + cm::static_string_view const TypeName; + cm::static_string_view const DefaultDirectoryProperty; + cm::static_string_view const DefaultPathProperty; + cm::static_string_view const DirectoryPrefix; + cm::static_string_view const PathPrefix; + cm::static_string_view const TypeDescription; + cm::static_string_view const DefaultDescription; + cm::static_string_view const ArbitraryDescription; + + FileSetEntries SelfEntries; + FileSetEntries InterfaceEntries; + + template + bool WriteProperties(cmTarget* tgt, cmTargetInternals* impl, + const std::string& prop, ValueType value, bool clear); + std::pair ReadProperties(cmTarget const* tgt, + cmTargetInternals const* impl, + const std::string& prop) const; + + void AddFileSet(const std::string& name, cmFileSetVisibility vis, + cmListFileBacktrace bt); +}; +} + class cmTargetInternals { public: @@ -206,13 +266,15 @@ public: std::vector> LinkInterfacePropertyEntries; std::vector> LinkInterfaceDirectPropertyEntries; std::vector> LinkInterfaceDirectExcludePropertyEntries; - std::vector> HeaderSetsEntries; - std::vector> InterfaceHeaderSetsEntries; std::vector> TLLCommands; std::map FileSets; cmListFileBacktrace Backtrace; + FileSetType HeadersFileSets; + + cmTargetInternals(); + bool CheckImportedLibName(std::string const& prop, std::string const& value) const; @@ -233,6 +295,123 @@ public: cm::string_view fileSetType) const; }; +cmTargetInternals::cmTargetInternals() + : HeadersFileSets("HEADERS"_s, "HEADER_DIRS"_s, "HEADER_SET"_s, + "HEADER_DIRS_"_s, "HEADER_SET_"_s, "Header"_s, + "The default header set"_s, "Header set"_s, + FileSetEntries("HEADER_SETS"_s), + FileSetEntries("INTERFACE_HEADER_SETS"_s)) +{ +} + +template +bool FileSetType::WriteProperties(cmTarget* tgt, cmTargetInternals* impl, + const std::string& prop, ValueType value, + bool clear) +{ + if (prop == this->DefaultDirectoryProperty) { + impl->AddDirectoryToFileSet(tgt, std::string(this->TypeName), value, + this->TypeName, this->DefaultDescription, + clear); + return true; + } + if (prop == this->DefaultPathProperty) { + impl->AddPathToFileSet(tgt, std::string(this->TypeName), value, + this->TypeName, this->DefaultDescription, clear); + return true; + } + if (cmHasPrefix(prop, this->DirectoryPrefix)) { + auto fileSetName = prop.substr(this->DirectoryPrefix.size()); + if (fileSetName.empty()) { + impl->Makefile->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat(this->ArbitraryDescription, " name cannot be empty.")); + } else { + impl->AddDirectoryToFileSet( + tgt, fileSetName, value, this->TypeName, + cmStrCat(this->ArbitraryDescription, " \"", fileSetName, "\""), clear); + } + return true; + } + if (cmHasPrefix(prop, this->PathPrefix)) { + auto fileSetName = prop.substr(this->PathPrefix.size()); + if (fileSetName.empty()) { + impl->Makefile->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat(this->ArbitraryDescription, " name cannot be empty.")); + } else { + impl->AddPathToFileSet( + tgt, fileSetName, value, this->TypeName, + cmStrCat(this->ArbitraryDescription, " \"", fileSetName, "\""), clear); + } + return true; + } + if (prop == this->SelfEntries.PropertyName) { + impl->Makefile->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat(this->SelfEntries.PropertyName, " property is read-only\n")); + return true; + } + if (prop == this->InterfaceEntries.PropertyName) { + impl->Makefile->IssueMessage(MessageType::FATAL_ERROR, + cmStrCat(this->InterfaceEntries.PropertyName, + " property is read-only\n")); + return true; + } + return false; +} + +std::pair FileSetType::ReadProperties( + cmTarget const* tgt, cmTargetInternals const* impl, + const std::string& prop) const +{ + bool did_read = false; + cmValue value = nullptr; + if (prop == this->DefaultDirectoryProperty) { + value = impl->GetFileSetDirectories(tgt, std::string(this->TypeName), + this->TypeName); + did_read = true; + } else if (prop == this->DefaultPathProperty) { + value = + impl->GetFileSetPaths(tgt, std::string(this->TypeName), this->TypeName); + did_read = true; + } else if (prop == this->SelfEntries.PropertyName) { + static std::string output; + output = cmJoin(this->SelfEntries.Entries, ";"_s); + value = cmValue(output); + did_read = true; + } else if (prop == this->InterfaceEntries.PropertyName) { + static std::string output; + output = cmJoin(this->InterfaceEntries.Entries, ";"_s); + value = cmValue(output); + did_read = true; + } else if (cmHasPrefix(prop, this->DirectoryPrefix)) { + std::string fileSetName = prop.substr(this->DirectoryPrefix.size()); + if (!fileSetName.empty()) { + value = impl->GetFileSetDirectories(tgt, fileSetName, this->TypeName); + } + did_read = true; + } else if (cmHasPrefix(prop, this->PathPrefix)) { + std::string fileSetName = prop.substr(this->PathPrefix.size()); + if (!fileSetName.empty()) { + value = impl->GetFileSetPaths(tgt, fileSetName, this->TypeName); + } + did_read = true; + } + return { did_read, value }; +} + +void FileSetType::AddFileSet(const std::string& name, cmFileSetVisibility vis, + cmListFileBacktrace bt) +{ + if (cmFileSetVisibilityIsForSelf(vis)) { + this->SelfEntries.Entries.emplace_back(name, bt); + } + if (cmFileSetVisibilityIsForInterface(vis)) { + this->InterfaceEntries.Entries.emplace_back(name, std::move(bt)); + } +} + namespace { #define SETUP_COMMON_LANGUAGE_PROPERTIES(lang) \ initProp(#lang "_COMPILER_LAUNCHER"); \ @@ -1177,12 +1356,12 @@ cmBTStringRange cmTarget::GetLinkInterfaceDirectExcludeEntries() const cmBTStringRange cmTarget::GetHeaderSetsEntries() const { - return cmMakeRange(this->impl->HeaderSetsEntries); + return cmMakeRange(this->impl->HeadersFileSets.SelfEntries.Entries); } cmBTStringRange cmTarget::GetInterfaceHeaderSetsEntries() const { - return cmMakeRange(this->impl->InterfaceHeaderSetsEntries); + return cmMakeRange(this->impl->HeadersFileSets.InterfaceEntries.Entries); } namespace { @@ -1214,10 +1393,6 @@ MAKE_PROP(BINARY_DIR); MAKE_PROP(SOURCE_DIR); MAKE_PROP(FALSE); MAKE_PROP(TRUE); -MAKE_PROP(HEADER_DIRS); -MAKE_PROP(HEADER_SET); -MAKE_PROP(HEADER_SETS); -MAKE_PROP(INTERFACE_HEADER_SETS); MAKE_PROP(INTERFACE_LINK_LIBRARIES); MAKE_PROP(INTERFACE_LINK_LIBRARIES_DIRECT); MAKE_PROP(INTERFACE_LINK_LIBRARIES_DIRECT_EXCLUDE); @@ -1392,7 +1567,9 @@ void cmTarget::StoreProperty(const std::string& prop, ValueType value) } } else if (cmHasLiteralPrefix(prop, "IMPORTED_LIBNAME") && !this->impl->CheckImportedLibName( - prop, value ? value : std::string{})) { + prop, + value ? value + : std::string{})) { // NOLINT(bugprone-branch-clone) /* error was reported by check method */ } else if (prop == propCUDA_PTX_COMPILATION && this->GetType() != cmStateEnums::OBJECT_LIBRARY) { @@ -1443,41 +1620,9 @@ void cmTarget::StoreProperty(const std::string& prop, ValueType value) } else { this->impl->LanguageStandardProperties.erase(prop); } - } else if (prop == propHEADER_DIRS) { - this->impl->AddDirectoryToFileSet(this, "HEADERS", value, "HEADERS"_s, - "The default header set"_s, true); - } else if (prop == propHEADER_SET) { - this->impl->AddPathToFileSet(this, "HEADERS", value, "HEADERS"_s, - "The default header set"_s, true); - } else if (cmHasLiteralPrefix(prop, "HEADER_DIRS_")) { - auto fileSetName = prop.substr(cmStrLen("HEADER_DIRS_")); - if (fileSetName.empty()) { - this->impl->Makefile->IssueMessage(MessageType::FATAL_ERROR, - "Header set name cannot be empty."); - return; - } - this->impl->AddDirectoryToFileSet( - this, fileSetName, value, "HEADERS"_s, - cmStrCat("Header set \"", fileSetName, "\""), true); - } else if (cmHasLiteralPrefix(prop, "HEADER_SET_")) { - auto fileSetName = prop.substr(cmStrLen("HEADER_SET_")); - if (fileSetName.empty()) { - this->impl->Makefile->IssueMessage(MessageType::FATAL_ERROR, - "Header set name cannot be empty."); - return; - } - this->impl->AddPathToFileSet(this, fileSetName, value, "HEADERS"_s, - cmStrCat("Header set \"", fileSetName, "\""), - true); - } else if (prop == propHEADER_SETS) { - this->impl->Makefile->IssueMessage(MessageType::FATAL_ERROR, - "HEADER_SETS property is read-only\n"); - return; - } else if (prop == propINTERFACE_HEADER_SETS) { - this->impl->Makefile->IssueMessage( - MessageType::FATAL_ERROR, - "INTERFACE_HEADER_SETS property is read-only\n"); - return; + } else if (this->impl->HeadersFileSets.WriteProperties( + this, this->impl.get(), prop, value, true)) { + /* Handled in the `if` condition. */ } else { this->impl->Properties.SetProperty(prop, value); } @@ -1588,41 +1733,9 @@ void cmTarget::AppendProperty(const std::string& prop, prop == "OBJC_STANDARD" || prop == "OBJCXX_STANDARD") { this->impl->Makefile->IssueMessage( MessageType::FATAL_ERROR, prop + " property may not be appended."); - } else if (prop == "HEADER_DIRS") { - this->impl->AddDirectoryToFileSet(this, "HEADERS", value, "HEADERS"_s, - "The default header set"_s, false); - } else if (cmHasLiteralPrefix(prop, "HEADER_DIRS_")) { - auto fileSetName = prop.substr(cmStrLen("HEADER_DIRS_")); - if (fileSetName.empty()) { - this->impl->Makefile->IssueMessage(MessageType::FATAL_ERROR, - "Header set name cannot be empty."); - return; - } - this->impl->AddDirectoryToFileSet( - this, fileSetName, value, "HEADERS"_s, - cmStrCat("Header set \"", fileSetName, "\""), false); - } else if (prop == "HEADER_SET") { - this->impl->AddPathToFileSet(this, "HEADERS", value, "HEADERS"_s, - "The default header set"_s, false); - } else if (cmHasLiteralPrefix(prop, "HEADER_SET_")) { - auto fileSetName = prop.substr(cmStrLen("HEADER_SET_")); - if (fileSetName.empty()) { - this->impl->Makefile->IssueMessage(MessageType::FATAL_ERROR, - "Header set name cannot be empty."); - return; - } - this->impl->AddPathToFileSet(this, fileSetName, value, "HEADERS"_s, - cmStrCat("Header set \"", fileSetName, "\""), - false); - } else if (prop == "HEADER_SETS") { - this->impl->Makefile->IssueMessage(MessageType::FATAL_ERROR, - "HEADER_SETS property is read-only\n"); - return; - } else if (prop == "INTERFACE_HEADER_SETS") { - this->impl->Makefile->IssueMessage( - MessageType::FATAL_ERROR, - "INTERFACE_HEADER_SETS property is read-only\n"); - return; + } else if (this->impl->HeadersFileSets.WriteProperties( + this, this->impl.get(), prop, value, false)) { + /* Handled in the `if` condition. */ } else { this->impl->Properties.AppendProperty(prop, value, asString); } @@ -1964,10 +2077,6 @@ cmValue cmTarget::GetProperty(const std::string& prop) const propBINARY_DIR, propSOURCE_DIR, propSOURCES, - propHEADER_DIRS, - propHEADER_SET, - propHEADER_SETS, - propINTERFACE_HEADER_SETS, propINTERFACE_LINK_LIBRARIES, propINTERFACE_LINK_LIBRARIES_DIRECT, propINTERFACE_LINK_LIBRARIES_DIRECT_EXCLUDE, @@ -2124,49 +2233,15 @@ cmValue cmTarget::GetProperty(const std::string& prop) const .GetDirectory() .GetCurrentSource()); } - if (prop == propHEADER_DIRS) { - return this->impl->GetFileSetDirectories(this, "HEADERS", "HEADERS"_s); - } - if (prop == propHEADER_SET) { - return this->impl->GetFileSetPaths(this, "HEADERS", "HEADERS"_s); - } - if (prop == propHEADER_SETS) { - std::vector set_names; - for (auto const& file_set : this->impl->FileSets) { - if (cmFileSetVisibilityIsForSelf(file_set.second.GetVisibility())) { - set_names.push_back(file_set.second.GetName()); - } - } - static std::string output; - output = cmJoin(set_names, ";"_s); - return cmValue(output); - } - if (prop == propINTERFACE_HEADER_SETS) { - std::vector set_names; - for (auto const& file_set : this->impl->FileSets) { - if (cmFileSetVisibilityIsForInterface( - file_set.second.GetVisibility())) { - set_names.push_back(file_set.second.GetName()); - } - } - static std::string output; - output = cmJoin(set_names, ";"_s); - return cmValue(output); - } } - if (cmHasLiteralPrefix(prop, "HEADER_DIRS_")) { - std::string fileSetName = prop.substr(cmStrLen("HEADER_DIRS_")); - if (fileSetName.empty()) { - return nullptr; - } - return this->impl->GetFileSetDirectories(this, fileSetName, "HEADERS"_s); - } - if (cmHasLiteralPrefix(prop, "HEADER_SET_")) { - std::string fileSetName = prop.substr(cmStrLen("HEADER_SET_")); - if (fileSetName.empty()) { - return nullptr; + + // Check fileset properties. + { + auto headers = + this->impl->HeadersFileSets.ReadProperties(this, this->impl.get(), prop); + if (headers.first) { + return headers.second; } - return this->impl->GetFileSetPaths(this, fileSetName, "HEADERS"_s); } cmValue retVal = this->impl->Properties.GetPropertyValue(prop); @@ -2441,13 +2516,9 @@ std::pair cmTarget::GetOrCreateFileSet( auto result = this->impl->FileSets.emplace( std::make_pair(name, cmFileSet(name, type, vis))); if (result.second) { - if (cmFileSetVisibilityIsForSelf(vis)) { - this->impl->HeaderSetsEntries.emplace_back( - name, this->impl->Makefile->GetBacktrace()); - } - if (cmFileSetVisibilityIsForInterface(vis)) { - this->impl->InterfaceHeaderSetsEntries.emplace_back( - name, this->impl->Makefile->GetBacktrace()); + auto bt = this->impl->Makefile->GetBacktrace(); + if (type == this->impl->HeadersFileSets.TypeName) { + this->impl->HeadersFileSets.AddFileSet(name, vis, std::move(bt)); } } return std::make_pair(&result.first->second, result.second); @@ -2481,7 +2552,7 @@ std::vector cmTarget::GetAllInterfaceFileSets() const } }; - appendEntries(this->impl->InterfaceHeaderSetsEntries); + appendEntries(this->impl->HeadersFileSets.InterfaceEntries.Entries); return result; } diff --git a/Tests/RunCMake/target_sources/FileSetImport.cmake b/Tests/RunCMake/target_sources/FileSetImport.cmake index c7acc5f..db82ca8 100644 --- a/Tests/RunCMake/target_sources/FileSetImport.cmake +++ b/Tests/RunCMake/target_sources/FileSetImport.cmake @@ -17,7 +17,7 @@ include("${export_build_dir}/export.cmake") include("${export_build_dir}/install/lib/cmake/export.cmake") assert_prop_eq(export::lib1 HEADER_SETS "") -assert_prop_eq(export::lib1 INTERFACE_HEADER_SETS "HEADERS;b;c;d;dir3;e;f;g") +assert_prop_eq(export::lib1 INTERFACE_HEADER_SETS "HEADERS;b;c;d;e;f;g;dir3") assert_prop_eq(export::lib1 HEADER_SET "${CMAKE_CURRENT_SOURCE_DIR}/error.c") assert_prop_eq(export::lib1 HEADER_SET_HEADERS "${CMAKE_CURRENT_SOURCE_DIR}/error.c") if (_multi_config) @@ -72,7 +72,7 @@ else () endif () assert_prop_eq(install::lib1 HEADER_SETS "") -assert_prop_eq(install::lib1 INTERFACE_HEADER_SETS "HEADERS;b;c;d;dir3;e;f;g") +assert_prop_eq(install::lib1 INTERFACE_HEADER_SETS "HEADERS;b;c;d;e;f;g;dir3") assert_prop_eq(install::lib1 HEADER_SET "${export_build_dir}/install/include/error.c") assert_prop_eq(install::lib1 HEADER_DIRS "${export_build_dir}/install/include") assert_prop_eq(install::lib1 HEADER_SET_HEADERS "${export_build_dir}/install/include/error.c") diff --git a/Tests/RunCMake/target_sources/FileSetProperties.cmake b/Tests/RunCMake/target_sources/FileSetProperties.cmake index a9a32b2..56cce08 100644 --- a/Tests/RunCMake/target_sources/FileSetProperties.cmake +++ b/Tests/RunCMake/target_sources/FileSetProperties.cmake @@ -57,7 +57,7 @@ assert_prop_eq(lib1 INCLUDE_DIRECTORIES "$;$;$") target_sources(lib1 PUBLIC FILE_SET HEADERS BASE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}" FILES h1.h) -assert_prop_eq(lib1 INTERFACE_HEADER_SETS "HEADERS;a;c;d") +assert_prop_eq(lib1 INTERFACE_HEADER_SETS "a;c;d;HEADERS") assert_prop_eq(lib1 HEADER_DIRS "${CMAKE_CURRENT_SOURCE_DIR}") assert_prop_eq(lib1 HEADER_SET "${CMAKE_CURRENT_SOURCE_DIR}/h1.h") assert_prop_eq(lib1 HEADER_DIRS_HEADERS "${CMAKE_CURRENT_SOURCE_DIR}") @@ -66,7 +66,7 @@ assert_prop_eq(lib1 INCLUDE_DIRECTORIES "$;$;$;$") target_sources(lib1 PUBLIC FILE_SET HEADERS FILES h2.h) -assert_prop_eq(lib1 INTERFACE_HEADER_SETS "HEADERS;a;c;d") +assert_prop_eq(lib1 INTERFACE_HEADER_SETS "a;c;d;HEADERS") assert_prop_eq(lib1 HEADER_DIRS "${CMAKE_CURRENT_SOURCE_DIR}") assert_prop_eq(lib1 HEADER_SET "${CMAKE_CURRENT_SOURCE_DIR}/h1.h;${CMAKE_CURRENT_SOURCE_DIR}/h2.h") assert_prop_eq(lib1 HEADER_DIRS_HEADERS "${CMAKE_CURRENT_SOURCE_DIR}") -- cgit v0.12