Merge topic 'autogen_reenable_implicit_include_dirs'

03dbb62d Autogen: Reenable passing compiler implicit include directories to moc
5f34bdc7 cmLocalGenerator: Refactor `GetIncludeDirectoriesImplicit` method
f16dfdf7

 cmLocalGenerator: Simplify `GetIncludeDirectories`

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !2864

Source/cmFileAPICodemodel.cxx

@@ -727,7 +727,7 @@ void Target::ProcessLanguage(std::string const& lang)
     lg->GetTargetDefines(this->GT, this->Config, lang);
   cd.SetDefines(defines);
   std::vector<BT<std::string>> includePathList =
-    lg->GetIncludeDirectories(this->GT, lang, this->Config, true);
+    lg->GetIncludeDirectories(this->GT, lang, this->Config);
   for (BT<std::string> const& i : includePathList) {
     cd.Includes.emplace_back(
       i, this->GT->IsSystemIncludeDirectory(i.Value, this->Config, lang));

Source/cmJsonObjects.cxx

@@ -578,7 +578,7 @@ static Json::Value DumpTarget(cmGeneratorTarget* target,
     lg->GetTargetDefines(target, config, lang, defines);
     ld.SetDefines(defines);
     std::vector<std::string> includePathList;
-    lg->GetIncludeDirectories(includePathList, target, lang, config, true);
+    lg->GetIncludeDirectories(includePathList, target, lang, config);
     for (std::string const& i : includePathList) {
       ld.IncludePathList.emplace_back(
         i, target->IsSystemIncludeDirectory(i, config, lang));

Source/cmLocalGenerator.cxx

@@ -881,31 +881,27 @@ void cmLocalGenerator::AddCompileOptions(std::string& flags,
   this->AddCompilerRequirementFlag(flags, target, lang);
 }
 
-void cmLocalGenerator::GetIncludeDirectories(std::vector<std::string>& dirs,
-                                             cmGeneratorTarget const* target,
-                                             const std::string& lang,
-                                             const std::string& config,
-                                             bool stripImplicitDirs,
-                                             bool appendAllImplicitDirs) const
-{
-  std::vector<BT<std::string>> tmp = this->GetIncludeDirectories(
-    target, lang, config, stripImplicitDirs, appendAllImplicitDirs);
-  dirs.reserve(tmp.size());
-  for (BT<std::string>& v : tmp) {
-    dirs.emplace_back(std::move(v.Value));
-  }
-}
-
-std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectories(
+std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectoriesImplicit(
   cmGeneratorTarget const* target, std::string const& lang,
   std::string const& config, bool stripImplicitDirs,
   bool appendAllImplicitDirs) const
 {
   std::vector<BT<std::string>> result;
-
   // Do not repeat an include path.
   std::set<std::string> emitted;
 
+  auto emitDir = [&result, &emitted](std::string const& dir) {
+    if (emitted.insert(dir).second) {
+      result.emplace_back(dir);
+    }
+  };
+
+  auto emitBT = [&result, &emitted](BT<std::string> const& dir) {
+    if (emitted.insert(dir.Value).second) {
+      result.emplace_back(dir);
+    }
+  };
+
   // When automatic include directories are requested for a build then
   // include the source and binary directories at the beginning of the
   // include path to approximate include file behavior for an
@@ -915,21 +911,9 @@ std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectories(
   // per-source-file include paths.
   if (this->Makefile->IsOn("CMAKE_INCLUDE_CURRENT_DIR")) {
     // Current binary directory
-    {
-      std::string binDir =
-        this->StateSnapshot.GetDirectory().GetCurrentBinary();
-      if (emitted.insert(binDir).second) {
-        result.emplace_back(std::move(binDir));
-      }
-    }
+    emitDir(this->StateSnapshot.GetDirectory().GetCurrentBinary());
     // Current source directory
-    {
-      std::string srcDir =
-        this->StateSnapshot.GetDirectory().GetCurrentSource();
-      if (emitted.insert(srcDir).second) {
-        result.emplace_back(std::move(srcDir));
-      }
-    }
+    emitDir(this->StateSnapshot.GetDirectory().GetCurrentSource());
   }
 
   if (!target) {
@@ -938,6 +922,11 @@ std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectories(
 
   // Implicit include directories
   std::vector<std::string> implicitDirs;
+  std::set<std::string> implicitSet;
+  // Checks if this is not an implicit include directory
+  auto notImplicit = [&implicitSet](std::string const& dir) {
+    return (implicitSet.find(dir) == implicitSet.end());
+  };
   {
     std::string rootPath;
     if (const char* sysrootCompile =
@@ -947,6 +936,7 @@ std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectories(
       rootPath = this->Makefile->GetSafeDefinition("CMAKE_SYSROOT");
     }
 
+    // Raw list of implicit include directories
     std::vector<std::string> impDirVec;
 
     // Get platform-wide implicit directories.
@@ -964,12 +954,11 @@ std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectories(
     }
 
     for (std::string const& i : impDirVec) {
-      {
-        std::string d = rootPath + i;
-        cmSystemTools::ConvertToUnixSlashes(d);
-        emitted.insert(std::move(d));
+      std::string imd = rootPath + i;
+      cmSystemTools::ConvertToUnixSlashes(imd);
+      if (implicitSet.insert(imd).second) {
+        implicitDirs.emplace_back(std::move(imd));
       }
-      implicitDirs.push_back(i);
     }
   }
 
@@ -982,30 +971,31 @@ std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectories(
   if (this->Makefile->IsOn("CMAKE_INCLUDE_DIRECTORIES_PROJECT_BEFORE")) {
     std::string const &topSourceDir = this->GetState()->GetSourceDirectory(),
                       &topBinaryDir = this->GetState()->GetBinaryDirectory();
-    for (BT<std::string> const& i : userDirs) {
+    for (BT<std::string> const& udr : userDirs) {
       // Emit this directory only if it is a subdirectory of the
       // top-level source or binary tree.
-      if (cmSystemTools::ComparePath(i.Value, topSourceDir) ||
-          cmSystemTools::ComparePath(i.Value, topBinaryDir) ||
-          cmSystemTools::IsSubDirectory(i.Value, topSourceDir) ||
-          cmSystemTools::IsSubDirectory(i.Value, topBinaryDir)) {
-        if (emitted.insert(i.Value).second) {
-          result.push_back(i);
+      if (cmSystemTools::ComparePath(udr.Value, topSourceDir) ||
+          cmSystemTools::ComparePath(udr.Value, topBinaryDir) ||
+          cmSystemTools::IsSubDirectory(udr.Value, topSourceDir) ||
+          cmSystemTools::IsSubDirectory(udr.Value, topBinaryDir)) {
+        if (notImplicit(udr.Value)) {
+          emitBT(udr);
         }
       }
     }
   }
 
-  // Construct the final ordered include directory list.
-  for (BT<std::string> const& i : userDirs) {
-    if (emitted.insert(i.Value).second) {
-      result.push_back(i);
+  // Emit remaining non implicit user direcories.
+  for (BT<std::string> const& udr : userDirs) {
+    if (notImplicit(udr.Value)) {
+      emitBT(udr);
     }
   }
 
+  // Sort result
   MoveSystemIncludesToEnd(result, config, lang, target);
 
-  // Add standard include directories for this language.
+  // Append standard include directories for this language.
   {
     std::vector<std::string> userStandardDirs;
     {
@@ -1016,26 +1006,27 @@ std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectories(
       cmSystemTools::ExpandListArgument(value, userStandardDirs);
     }
     userDirs.reserve(userDirs.size() + userStandardDirs.size());
-    for (std::string& d : userStandardDirs) {
-      cmSystemTools::ConvertToUnixSlashes(d);
-      result.emplace_back(d);
-      userDirs.emplace_back(std::move(d));
+    for (std::string& usd : userStandardDirs) {
+      cmSystemTools::ConvertToUnixSlashes(usd);
+      if (notImplicit(usd)) {
+        emitDir(usd);
+      }
+      userDirs.emplace_back(std::move(usd));
     }
   }
 
+  // Append compiler implicit include directories
   if (!stripImplicitDirs) {
-    // Append only implicit directories that were requested by the user
-    for (std::string const& i : implicitDirs) {
-      if (std::find(userDirs.begin(), userDirs.end(), i) != userDirs.end()) {
-        result.emplace_back(i);
+    // Append implicit directories that were requested by the user only
+    for (BT<std::string> const& udr : userDirs) {
+      if (!notImplicit(udr.Value)) {
+        emitBT(udr);
       }
     }
-    // Append remaining implicit directories on demand
+    // Append remaining implicit directories (on demand)
     if (appendAllImplicitDirs) {
-      for (std::string const& i : implicitDirs) {
-        if (std::find(result.begin(), result.end(), i) == result.end()) {
-          result.emplace_back(i);
-        }
+      for (std::string& imd : implicitDirs) {
+        emitDir(imd);
       }
     }
   }
@@ -1043,6 +1034,34 @@ std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectories(
   return result;
 }
 
+void cmLocalGenerator::GetIncludeDirectoriesImplicit(
+  std::vector<std::string>& dirs, cmGeneratorTarget const* target,
+  const std::string& lang, const std::string& config, bool stripImplicitDirs,
+  bool appendAllImplicitDirs) const
+{
+  std::vector<BT<std::string>> tmp = this->GetIncludeDirectoriesImplicit(
+    target, lang, config, stripImplicitDirs, appendAllImplicitDirs);
+  dirs.reserve(dirs.size() + tmp.size());
+  for (BT<std::string>& v : tmp) {
+    dirs.emplace_back(std::move(v.Value));
+  }
+}
+
+std::vector<BT<std::string>> cmLocalGenerator::GetIncludeDirectories(
+  cmGeneratorTarget const* target, std::string const& lang,
+  std::string const& config) const
+{
+  return this->GetIncludeDirectoriesImplicit(target, lang, config);
+}
+
+void cmLocalGenerator::GetIncludeDirectories(std::vector<std::string>& dirs,
+                                             cmGeneratorTarget const* target,
+                                             const std::string& lang,
+                                             const std::string& config) const
+{
+  this->GetIncludeDirectoriesImplicit(dirs, target, lang, config);
+}
+
 void cmLocalGenerator::GetStaticLibraryFlags(std::string& flags,
                                              std::string const& config,
                                              std::string const& linkLanguage,

Source/cmLocalGenerator.h

@@ -239,22 +239,46 @@ public:
     return true;
   }
 
-  /** @brief Get the include directories for the current makefile and language.
+  /** @brief Get the include directories for the current makefile and language
+   * and optional the compiler implicit include directories.
+   *
    * @arg stripImplicitDirs Strip all directories found in
    *      CMAKE_<LANG>_IMPLICIT_INCLUDE_DIRECTORIES from the result.
    * @arg appendAllImplicitDirs Append all directories found in
    *      CMAKE_<LANG>_IMPLICIT_INCLUDE_DIRECTORIES to the result.
    */
+  std::vector<BT<std::string>> GetIncludeDirectoriesImplicit(
+    cmGeneratorTarget const* target, std::string const& lang = "C",
+    std::string const& config = "", bool stripImplicitDirs = true,
+    bool appendAllImplicitDirs = false) const;
+
+  /** @brief Get the include directories for the current makefile and language
+   * and optional the compiler implicit include directories.
+   *
+   * @arg dirs Directories are appended to this list
+   */
+  void GetIncludeDirectoriesImplicit(std::vector<std::string>& dirs,
+                                     cmGeneratorTarget const* target,
+                                     const std::string& lang = "C",
+                                     const std::string& config = "",
+                                     bool stripImplicitDirs = true,
+                                     bool appendAllImplicitDirs = false) const;
+
+  /** @brief Get the include directories for the current makefile and language.
+   * @arg dirs Include directories are appended to this list
+   */
   void GetIncludeDirectories(std::vector<std::string>& dirs,
                              cmGeneratorTarget const* target,
                              const std::string& lang = "C",
-                             const std::string& config = "",
-                             bool stripImplicitDirs = true,
-                             bool appendAllImplicitDirs = false) const;
+                             const std::string& config = "") const;
+
+  /** @brief Get the include directories for the current makefile and language.
+   * @return The include directory list
+   */
   std::vector<BT<std::string>> GetIncludeDirectories(
     cmGeneratorTarget const* target, std::string const& lang = "C",
-    std::string const& config = "", bool stripImplicitDirs = true,
-    bool appendAllImplicitDirs = false) const;
+    std::string const& config = "") const;
+
   void AddCompileOptions(std::string& flags, cmGeneratorTarget* target,
                          const std::string& lang, const std::string& config);
 

Source/cmQtAutoGenInitializer.cxx

@@ -520,19 +520,16 @@ bool cmQtAutoGenInitializer::InitMoc()
 
   // Moc includes
   {
-    // We need to disable this until we have all implicit includes available.
-    // See issue #18669.
-    // bool const appendImplicit = (this->QtVersion.Major >= 5);
-
+    bool const appendImplicit = (this->QtVersion.Major >= 5);
     auto GetIncludeDirs =
-      [this, localGen](std::string const& cfg) -> std::vector<std::string> {
-      bool const appendImplicit = false;
+      [this, localGen,
+       appendImplicit](std::string const& cfg) -> std::vector<std::string> {
       // Get the include dirs for this target, without stripping the implicit
       // include dirs off, see
       // https://gitlab.kitware.com/cmake/cmake/issues/13667
       std::vector<std::string> dirs;
-      localGen->GetIncludeDirectories(dirs, this->Target, "CXX", cfg, false,
-                                      appendImplicit);
+      localGen->GetIncludeDirectoriesImplicit(dirs, this->Target, "CXX", cfg,
+                                              false, appendImplicit);
       return dirs;
     };
 

Tests/QtAutogen/MocOsMacros/TestClass.cpp

 #include "TestClass.hpp"
 #include <iostream>
 
+void TestClass::open()
+{
+  std::cout << "open\n";
+}
+
 // -- Mac
 #ifndef Q_OS_MAC
 void TestClass::MacNotDef()

Tests/QtAutogen/MocOsMacros/TestClass.hpp

@@ -3,12 +3,17 @@
 
 #include <QObject>
 #include <QtGlobal>
+// include qplatformdefs.h for #18669
+#include <qplatformdefs.h>
 
 class TestClass : public QObject
 {
   Q_OBJECT
 public Q_SLOTS:
 
+  // Method named "open" to test if #18669 is fixed
+  void open();
+
 // -- Mac
 #ifndef Q_OS_MAC
   void MacNotDef();

Tests/QtAutogen/Tests.cmake

@@ -39,8 +39,7 @@ endif()
 # Qt5 only tests
 if(QT_TEST_VERSION GREATER 4)
   ADD_AUTOGEN_TEST(MocMacroName mocMacroName)
-  # Disabled for issue #18669
-  #ADD_AUTOGEN_TEST(MocOsMacros)
+  ADD_AUTOGEN_TEST(MocOsMacros)
   ADD_AUTOGEN_TEST(RerunMocPlugin)
   if(APPLE)
     ADD_AUTOGEN_TEST(MacOsFW)