Commit b0b820ea authored by Sebastian Holtermann's avatar Sebastian Holtermann
Browse files

cmLocalGenerator: Extend the functionality of ``GetIncludeDirectories()``

What ``cmLocalGenerator::GetIncludeDirectories`` does
-----------------------------------------------------

In general it concatenates the

1. ``target->GetIncludeDirectories(LANG)`` and the
2. ``CMAKE_<LANG>_STANDARD_INCLUDE_DIRECTORIES``.

Additionally it performs some sorting and special treatment of the

- ``CMAKE_<LANG>_IMPLICIT_INCLUDE_DIRECTORIES``.

By default all ``CMAKE_<LANG>_IMPLICIT_INCLUDE_DIRECTORIES`` are stripped from
the result list.

When explicitly requested (by setting ``stripImplicitInclDirs=false``) *some*
implicit directories are appended to the result list.  The implicit directories
that *are* appended are those that were requested to be included by

1. ``target->GetIncludeDirectories(LANG)`` or
2. ``CMAKE_<LANG>_STANDARD_INCLUDE_DIRECTORIES``.

All other implicit directories are still stripped from the result list.

The reason to not simply append all implicit directories is that Qt4's moc has
problems to parse some headers that might be found in the implicit system
include directories (See commit d2536579
and
[QTBUG-28045](https://bugreports.qt.io/browse/QTBUG-28045)
).
That has been solved in Qt5's moc though.

Extension request to ``cmLocalGenerator::GetIncludeDirectories``
----------------------------------------------------------------

For Qt5's moc we like to have an option that allows to append *all* implict
include directories to the result list, not just those that were user requested.

Changes to ``cmLocalGenerator::GetIncludeDirectories``
------------------------------------------------------

- Shorten the function parameter name ``stripImplicitInclDirs`` to
  ``stripImplicitDirs``.

- Add new boolean function parameter ``appendAllImplicitDirs``
  with a default value ``false``.

The old default behavior of the function stays the same, but a specialized
behavior can be requested by AUTOMOC for Qt4/Qt5 respectively.
parent 1f36652e
......@@ -844,7 +844,8 @@ void cmLocalGenerator::GetIncludeDirectories(std::vector<std::string>& dirs,
cmGeneratorTarget const* target,
const std::string& lang,
const std::string& config,
bool stripImplicitInclDirs) const
bool stripImplicitDirs,
bool appendAllImplicitDirs) const
{
// Do not repeat an include path.
std::set<std::string> emitted;
......@@ -898,12 +899,12 @@ void cmLocalGenerator::GetIncludeDirectories(std::vector<std::string>& dirs,
std::vector<std::string> impDirVec;
cmSystemTools::ExpandListArgument(value, impDirVec);
for (std::string const& i : impDirVec) {
std::string d = rootPath + i;
cmSystemTools::ConvertToUnixSlashes(d);
emitted.insert(std::move(d));
if (!stripImplicitInclDirs) {
implicitDirs.push_back(i);
{
std::string d = rootPath + i;
cmSystemTools::ConvertToUnixSlashes(d);
emitted.insert(std::move(d));
}
implicitDirs.push_back(i);
}
}
}
......@@ -958,9 +959,21 @@ void cmLocalGenerator::GetIncludeDirectories(std::vector<std::string>& dirs,
}
}
for (std::string const& i : implicitDirs) {
if (std::find(userDirs.begin(), userDirs.end(), i) != userDirs.end()) {
dirs.push_back(i);
if (!stripImplicitDirs) {
if (!appendAllImplicitDirs) {
// Append only those implicit directories that were requested by the user
for (std::string const& i : implicitDirs) {
if (std::find(userDirs.begin(), userDirs.end(), i) != userDirs.end()) {
dirs.push_back(i);
}
}
} else {
// Append all implicit directories
for (std::string const& i : implicitDirs) {
if (std::find(dirs.begin(), dirs.end(), i) == dirs.end()) {
dirs.push_back(i);
}
}
}
}
}
......
......@@ -237,12 +237,18 @@ public:
return true;
}
/** Get the include flags for the current makefile and language. */
/** @brief Get the include directories for the current makefile and language.
* @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.
*/
void GetIncludeDirectories(std::vector<std::string>& dirs,
cmGeneratorTarget const* target,
const std::string& lang = "C",
const std::string& config = "",
bool stripImplicitInclDirs = true) const;
bool stripImplicitDirs = true,
bool appendAllImplicitDirs = false) const;
void AddCompileOptions(std::string& flags, cmGeneratorTarget* target,
const std::string& lang, const std::string& config);
void AddCompileDefinitions(std::set<std::string>& defines,
......
  • mentioned in commit 0ff961f1

    Toggle commit list
  • Is there a reason this change wasn't wrapped behind a condition or policy? It changes behavior in a way that can break clients. I'm pretty sure this is the change that caused the issue reported here -https://reviews.llvm.org/D69973.

    While it may not be common to explicitly provide paths that are also in the implicit include directories, it is possible (and sometimes necessary), and it seems to me that this patch would break that use case.

  • @llvm-beanz please open an issue rather than commenting on an already-merged commit.

  • @llvm-beanz this may be #19227.

  • @brad.king yep, that looks like roughly the same issue. Thanks.

Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment