cmFileSet: store visibility with the fileset

The visibility is intrinsic to the fileset, so store it with it. This
avoids recalculating it on every addition to the fileset.

Source/cmFileSet.cxx

@@ -7,19 +7,79 @@
 #include <utility>
 #include <vector>
 
+#include <cmext/string_view>
+
 #include "cmsys/RegularExpression.hxx"
 
 #include "cmGeneratorExpression.h"
 #include "cmListFileCache.h"
 #include "cmLocalGenerator.h"
+#include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
 #include "cmake.h"
 
-cmFileSet::cmFileSet(std::string name, std::string type)
+cm::static_string_view cmFileSetVisibilityToName(cmFileSetVisibility vis)
+{
+  switch (vis) {
+    case cmFileSetVisibility::Interface:
+      return "INTERFACE"_s;
+    case cmFileSetVisibility::Public:
+      return "PUBLIC"_s;
+    case cmFileSetVisibility::Private:
+      return "PRIVATE"_s;
+  }
+  return ""_s;
+}
+
+cmFileSetVisibility cmFileSetVisibilityFromName(cm::string_view name,
+                                                cmMakefile* mf)
+{
+  if (name == "INTERFACE"_s) {
+    return cmFileSetVisibility::Interface;
+  }
+  if (name == "PUBLIC"_s) {
+    return cmFileSetVisibility::Public;
+  }
+  if (name == "PRIVATE"_s) {
+    return cmFileSetVisibility::Private;
+  }
+  mf->IssueMessage(
+    MessageType::FATAL_ERROR,
+    cmStrCat("File set visibility \"", name, "\" is not valid."));
+  return cmFileSetVisibility::Private;
+}
+
+bool cmFileSetVisibilityIsForSelf(cmFileSetVisibility vis)
+{
+  switch (vis) {
+    case cmFileSetVisibility::Interface:
+      return false;
+    case cmFileSetVisibility::Public:
+    case cmFileSetVisibility::Private:
+      return true;
+  }
+  return false;
+}
+
+bool cmFileSetVisibilityIsForInterface(cmFileSetVisibility vis)
+{
+  switch (vis) {
+    case cmFileSetVisibility::Interface:
+    case cmFileSetVisibility::Public:
+      return true;
+    case cmFileSetVisibility::Private:
+      return false;
+  }
+  return false;
+}
+
+cmFileSet::cmFileSet(std::string name, std::string type,
+                     cmFileSetVisibility visibility)
   : Name(std::move(name))
   , Type(std::move(type))
+  , Visibility(visibility)
 {
 }
 

Source/cmFileSet.h

@@ -7,20 +7,38 @@
 #include <string>
 #include <vector>
 
+#include <cm/string_view>
+#include <cmext/string_view>
+
 #include "cmListFileCache.h"
 
 class cmCompiledGeneratorExpression;
 struct cmGeneratorExpressionDAGChecker;
 class cmGeneratorTarget;
 class cmLocalGenerator;
+class cmMakefile;
+
+enum class cmFileSetVisibility
+{
+  Private,
+  Public,
+  Interface,
+};
+cm::static_string_view cmFileSetVisibilityToName(cmFileSetVisibility vis);
+cmFileSetVisibility cmFileSetVisibilityFromName(cm::string_view name,
+                                                cmMakefile* mf);
+bool cmFileSetVisibilityIsForSelf(cmFileSetVisibility vis);
+bool cmFileSetVisibilityIsForInterface(cmFileSetVisibility vis);
 
 class cmFileSet
 {
 public:
-  cmFileSet(std::string name, std::string type);
+  cmFileSet(std::string name, std::string type,
+            cmFileSetVisibility visibility);
 
   const std::string& GetName() const { return this->Name; }
   const std::string& GetType() const { return this->Type; }
+  cmFileSetVisibility GetVisibility() const { return this->Visibility; }
 
   void ClearDirectoryEntries();
   void AddDirectoryEntry(BT<std::string> directories);
@@ -61,6 +79,7 @@ public:
 private:
   std::string Name;
   std::string Type;
+  cmFileSetVisibility Visibility;
   std::vector<BT<std::string>> DirectoryEntries;
   std::vector<BT<std::string>> FileEntries;
 };

Source/cmTarget.cxx

@@ -2342,10 +2342,10 @@ cmFileSet* cmTarget::GetFileSet(const std::string& name)
 }
 
 std::pair<cmFileSet*, bool> cmTarget::GetOrCreateFileSet(
-  const std::string& name, const std::string& type)
+  const std::string& name, const std::string& type, cmFileSetVisibility vis)
 {
-  auto result =
-    this->impl->FileSets.emplace(std::make_pair(name, cmFileSet(name, type)));
+  auto result = this->impl->FileSets.emplace(
+    std::make_pair(name, cmFileSet(name, type, vis)));
   return std::make_pair(&result.first->second, result.second);
 }
 

Source/cmTarget.h

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "cmAlgorithms.h"
+#include "cmFileSet.h"
 #include "cmPolicies.h"
 #include "cmStateTypes.h"
 #include "cmStringAlgorithms.h"
@@ -19,7 +20,6 @@
 #include "cmValue.h"
 
 class cmCustomCommand;
-class cmFileSet;
 class cmGlobalGenerator;
 class cmInstallTargetGenerator;
 class cmListFileBacktrace;
@@ -285,7 +285,8 @@ public:
   const cmFileSet* GetFileSet(const std::string& name) const;
   cmFileSet* GetFileSet(const std::string& name);
   std::pair<cmFileSet*, bool> GetOrCreateFileSet(const std::string& name,
-                                                 const std::string& type);
+                                                 const std::string& type,
+                                                 cmFileSetVisibility vis);
 
   std::vector<std::string> GetAllInterfaceFileSets() const;
 

Source/cmTargetSourcesCommand.cxx

@@ -2,7 +2,6 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmTargetSourcesCommand.h"
 
-#include <algorithm>
 #include <sstream>
 #include <utility>
 
@@ -239,7 +238,11 @@ bool TargetSourcesImpl::HandleOneFileSet(
     (args.Type.empty() && args.FileSet[0] >= 'A' && args.FileSet[0] <= 'Z');
   std::string type = isDefault ? args.FileSet : args.Type;
 
-  auto fileSet = this->Target->GetOrCreateFileSet(args.FileSet, type);
+  cmFileSetVisibility visibility =
+    cmFileSetVisibilityFromName(scope, this->Makefile);
+
+  auto fileSet =
+    this->Target->GetOrCreateFileSet(args.FileSet, type, visibility);
   if (fileSet.second) {
     if (!isDefault) {
       if (!cmFileSet::IsValidName(args.FileSet)) {
@@ -279,37 +282,11 @@ bool TargetSourcesImpl::HandleOneFileSet(
       return false;
     }
 
-    std::string existingScope = "PRIVATE";
-
-    auto const fileSetsProperty = cmTarget::GetFileSetsPropertyName(type);
-    auto const interfaceFileSetsProperty =
-      cmTarget::GetInterfaceFileSetsPropertyName(type);
-    std::vector<std::string> fileSets;
-    std::vector<std::string> interfaceFileSets;
-    cmExpandList(this->Target->GetSafeProperty(fileSetsProperty), fileSets);
-    cmExpandList(this->Target->GetSafeProperty(interfaceFileSetsProperty),
-                 interfaceFileSets);
-
-    if (std::find(interfaceFileSets.begin(), interfaceFileSets.end(),
-                  args.FileSet) != interfaceFileSets.end()) {
-      existingScope = "INTERFACE";
-    }
-    if (std::find(fileSets.begin(), fileSets.end(), args.FileSet) !=
-        fileSets.end()) {
-      if (existingScope == "INTERFACE"_s) {
-        existingScope = "PUBLIC";
-      }
-    } else if (existingScope != "INTERFACE"_s) {
-      this->SetError(cmStrCat("File set \"", args.FileSet, "\" is not in ",
-                              fileSetsProperty, " or ",
-                              interfaceFileSetsProperty));
-      return false;
-    }
-
-    if (scope != existingScope) {
+    if (visibility != fileSet.first->GetVisibility()) {
       this->SetError(
         cmStrCat("Scope ", scope, " for file set \"", args.FileSet,
-                 "\" does not match original scope ", existingScope));
+                 "\" does not match original scope ",
+                 cmFileSetVisibilityToName(fileSet.first->GetVisibility())));
       return false;
     }
   }
@@ -330,11 +307,11 @@ bool TargetSourcesImpl::HandleOneFileSet(
       for (auto const& dir : cmExpandedList(baseDirectories)) {
         auto interfaceDirectoriesGenex =
           cmStrCat("$<BUILD_INTERFACE:", dir, ">");
-        if (scope == "PRIVATE"_s || scope == "PUBLIC"_s) {
+        if (cmFileSetVisibilityIsForSelf(visibility)) {
           this->Target->AppendProperty("INCLUDE_DIRECTORIES",
                                        interfaceDirectoriesGenex);
         }
-        if (scope == "INTERFACE"_s || scope == "PUBLIC"_s) {
+        if (cmFileSetVisibilityIsForInterface(visibility)) {
           this->Target->AppendProperty("INTERFACE_INCLUDE_DIRECTORIES",
                                        interfaceDirectoriesGenex);
         }