Don't duplicate -D defines sent to the compiler.

There is no need to do so. Be consistent with include directories and
ensure uniqueness.

This requires changing the API of the cmLocalGenerator::AppendDefines
method, and changing the generators to match.

The test unfortunately can't test for uniqueness, but it at least verifies
that nothing gets lost.
This commit is contained in:
Stephen Kelly 2012-08-09 09:44:15 +02:00
parent d46f8afae9
commit 3dae652b4e
11 changed files with 211 additions and 96 deletions

View File

@ -2101,9 +2101,8 @@ void cmLocalGenerator::AppendFlags(std::string& flags,
}
//----------------------------------------------------------------------------
void cmLocalGenerator::AppendDefines(std::string& defines,
const char* defines_list,
const char* lang)
void cmLocalGenerator::AppendDefines(std::set<std::string>& defines,
const char* defines_list)
{
// Short-circuit if there are no definitions.
if(!defines_list)
@ -2115,12 +2114,23 @@ void cmLocalGenerator::AppendDefines(std::string& defines,
std::vector<std::string> defines_vec;
cmSystemTools::ExpandListArgument(defines_list, defines_vec);
// Short-circuit if there are no definitions.
if(defines_vec.empty())
for(std::vector<std::string>::const_iterator di = defines_vec.begin();
di != defines_vec.end(); ++di)
{
return;
// Skip unsupported definitions.
if(!this->CheckDefinition(*di))
{
continue;
}
defines.insert(*di);
}
}
//----------------------------------------------------------------------------
void cmLocalGenerator::JoinDefines(const std::set<std::string>& defines,
std::string &definesString,
const char* lang)
{
// Lookup the define flag for the current language.
std::string dflag = "-D";
if(lang)
@ -2135,23 +2145,13 @@ void cmLocalGenerator::AppendDefines(std::string& defines,
}
}
// Add each definition to the command line with appropriate escapes.
const char* dsep = defines.empty()? "" : " ";
for(std::vector<std::string>::const_iterator di = defines_vec.begin();
di != defines_vec.end(); ++di)
std::set<std::string>::const_iterator defineIt = defines.begin();
const std::set<std::string>::const_iterator defineEnd = defines.end();
const char* itemSeparator = definesString.empty() ? "" : " ";
for( ; defineIt != defineEnd; ++defineIt)
{
// Skip unsupported definitions.
if(!this->CheckDefinition(*di))
{
continue;
}
// Separate from previous definitions.
defines += dsep;
dsep = " ";
// Append the definition with proper escaping.
defines += dflag;
std::string def = dflag;
if(this->WatcomWMake)
{
// The Watcom compiler does its own command line parsing instead
@ -2164,27 +2164,30 @@ void cmLocalGenerator::AppendDefines(std::string& defines,
// command line without any escapes. However we still have to
// get the '$' and '#' characters through WMake as '$$' and
// '$#'.
for(const char* c = di->c_str(); *c; ++c)
for(const char* c = defineIt->c_str(); *c; ++c)
{
if(*c == '$' || *c == '#')
{
defines += '$';
def += '$';
}
defines += *c;
def += *c;
}
}
else
{
// Make the definition appear properly on the command line. Use
// -DNAME="value" instead of -D"NAME=value" to help VS6 parser.
std::string::size_type eq = di->find("=");
defines += di->substr(0, eq);
if(eq != di->npos)
std::string::size_type eq = defineIt->find("=");
def += defineIt->substr(0, eq);
if(eq != defineIt->npos)
{
defines += "=";
defines += this->EscapeForShell(di->c_str() + eq + 1, true);
def += "=";
def += this->EscapeForShell(defineIt->c_str() + eq + 1, true);
}
}
definesString += itemSeparator;
itemSeparator = " ";
definesString += def;
}
}

View File

@ -154,8 +154,14 @@ public:
* Encode a list of preprocessor definitions for the compiler
* command line.
*/
void AppendDefines(std::string& defines, const char* defines_list,
const char* lang);
void AppendDefines(std::set<std::string>& defines,
const char* defines_list);
/**
* Join a set of defines into a definesString with a space separator.
*/
void JoinDefines(const std::set<std::string>& defines,
std::string &definesString,
const char* lang);
/** Lookup and append options associated with a particular feature. */
void AppendFeatureOptions(std::string& flags, const char* lang,

View File

@ -418,25 +418,42 @@ void cmLocalVisualStudio6Generator
// Add per-source and per-configuration preprocessor definitions.
std::map<cmStdString, cmStdString> cdmap;
this->AppendDefines(compileFlags,
(*sf)->GetProperty("COMPILE_DEFINITIONS"), lang);
{
std::set<std::string> targetCompileDefinitions;
this->AppendDefines(targetCompileDefinitions,
(*sf)->GetProperty("COMPILE_DEFINITIONS"));
this->JoinDefines(targetCompileDefinitions, compileFlags, lang);
}
if(const char* cdefs = (*sf)->GetProperty("COMPILE_DEFINITIONS_DEBUG"))
{
this->AppendDefines(cdmap["DEBUG"], cdefs, lang);
std::set<std::string> debugCompileDefinitions;
this->AppendDefines(debugCompileDefinitions, cdefs);
this->JoinDefines(debugCompileDefinitions, cdmap["DEBUG"], lang);
}
if(const char* cdefs = (*sf)->GetProperty("COMPILE_DEFINITIONS_RELEASE"))
{
this->AppendDefines(cdmap["RELEASE"], cdefs, lang);
std::set<std::string> releaseCompileDefinitions;
this->AppendDefines(releaseCompileDefinitions, cdefs);
this->JoinDefines(releaseCompileDefinitions, cdmap["RELEASE"], lang);
}
if(const char* cdefs =
(*sf)->GetProperty("COMPILE_DEFINITIONS_MINSIZEREL"))
{
this->AppendDefines(cdmap["MINSIZEREL"], cdefs, lang);
std::set<std::string> minsizerelCompileDefinitions;
this->AppendDefines(minsizerelCompileDefinitions, cdefs);
this->JoinDefines(minsizerelCompileDefinitions, cdmap["MINSIZEREL"],
lang);
}
if(const char* cdefs =
(*sf)->GetProperty("COMPILE_DEFINITIONS_RELWITHDEBINFO"))
{
this->AppendDefines(cdmap["RELWITHDEBINFO"], cdefs, lang);
std::set<std::string> relwithdebinfoCompileDefinitions;
this->AppendDefines(relwithdebinfoCompileDefinitions, cdefs);
this->JoinDefines(relwithdebinfoCompileDefinitions,
cdmap["RELWITHDEBINFO"], lang);
}
bool excludedFromBuild =
@ -1653,43 +1670,56 @@ void cmLocalVisualStudio6Generator
}
// Add per-target and per-configuration preprocessor definitions.
std::set<std::string> definesSet;
std::set<std::string> debugDefinesSet;
std::set<std::string> releaseDefinesSet;
std::set<std::string> minsizeDefinesSet;
std::set<std::string> debugrelDefinesSet;
this->AppendDefines(
definesSet,
this->Makefile->GetProperty("COMPILE_DEFINITIONS"));
this->AppendDefines(
debugDefinesSet,
this->Makefile->GetProperty("COMPILE_DEFINITIONS_DEBUG"));
this->AppendDefines(
releaseDefinesSet,
this->Makefile->GetProperty("COMPILE_DEFINITIONS_RELEASE"));
this->AppendDefines(
minsizeDefinesSet,
this->Makefile->GetProperty("COMPILE_DEFINITIONS_MINSIZEREL"));
this->AppendDefines(
debugrelDefinesSet,
this->Makefile->GetProperty("COMPILE_DEFINITIONS_RELWITHDEBINFO"));
this->AppendDefines(
definesSet,
target.GetProperty("COMPILE_DEFINITIONS"));
this->AppendDefines(
debugDefinesSet,
target.GetProperty("COMPILE_DEFINITIONS_DEBUG"));
this->AppendDefines(
releaseDefinesSet,
target.GetProperty("COMPILE_DEFINITIONS_RELEASE"));
this->AppendDefines(
minsizeDefinesSet,
target.GetProperty("COMPILE_DEFINITIONS_MINSIZEREL"));
this->AppendDefines(
debugrelDefinesSet,
target.GetProperty("COMPILE_DEFINITIONS_RELWITHDEBINFO"));
std::string defines = " ";
std::string debugDefines = " ";
std::string releaseDefines = " ";
std::string minsizeDefines = " ";
std::string debugrelDefines = " ";
this->AppendDefines(
defines,
this->Makefile->GetProperty("COMPILE_DEFINITIONS"), 0);
this->AppendDefines(
debugDefines,
this->Makefile->GetProperty("COMPILE_DEFINITIONS_DEBUG"),0);
this->AppendDefines(
releaseDefines,
this->Makefile->GetProperty("COMPILE_DEFINITIONS_RELEASE"), 0);
this->AppendDefines(
minsizeDefines,
this->Makefile->GetProperty("COMPILE_DEFINITIONS_MINSIZEREL"), 0);
this->AppendDefines(
debugrelDefines,
this->Makefile->GetProperty("COMPILE_DEFINITIONS_RELWITHDEBINFO"), 0);
this->JoinDefines(definesSet, defines, 0);
this->JoinDefines(debugDefinesSet, debugDefines, 0);
this->JoinDefines(releaseDefinesSet, releaseDefines, 0);
this->JoinDefines(minsizeDefinesSet, minsizeDefines, 0);
this->JoinDefines(debugrelDefinesSet, debugrelDefines, 0);
this->AppendDefines(
defines,
target.GetProperty("COMPILE_DEFINITIONS"), 0);
this->AppendDefines(
debugDefines,
target.GetProperty("COMPILE_DEFINITIONS_DEBUG"), 0);
this->AppendDefines(
releaseDefines,
target.GetProperty("COMPILE_DEFINITIONS_RELEASE"), 0);
this->AppendDefines(
minsizeDefines,
target.GetProperty("COMPILE_DEFINITIONS_MINSIZEREL"), 0);
this->AppendDefines(
debugrelDefines,
target.GetProperty("COMPILE_DEFINITIONS_RELWITHDEBINFO"), 0);
flags += defines;
flagsDebug += debugDefines;
flagsRelease += releaseDefines;

View File

@ -292,28 +292,31 @@ std::string cmMakefileTargetGenerator::GetDefines(const std::string &l)
ByLanguageMap::iterator i = this->DefinesByLanguage.find(l);
if (i == this->DefinesByLanguage.end())
{
std::string defines;
std::set<std::string> defines;
const char *lang = l.c_str();
// Add the export symbol definition for shared library objects.
if(const char* exportMacro = this->Target->GetExportMacro())
{
this->LocalGenerator->AppendDefines(defines, exportMacro, lang);
this->LocalGenerator->AppendDefines(defines, exportMacro);
}
// Add preprocessor definitions for this target and configuration.
this->LocalGenerator->AppendDefines
(defines, this->Makefile->GetProperty("COMPILE_DEFINITIONS"), lang);
(defines, this->Makefile->GetProperty("COMPILE_DEFINITIONS"));
this->LocalGenerator->AppendDefines
(defines, this->Target->GetProperty("COMPILE_DEFINITIONS"), lang);
(defines, this->Target->GetProperty("COMPILE_DEFINITIONS"));
std::string defPropName = "COMPILE_DEFINITIONS_";
defPropName +=
cmSystemTools::UpperCase(this->LocalGenerator->ConfigurationName);
this->LocalGenerator->AppendDefines
(defines, this->Makefile->GetProperty(defPropName.c_str()), lang);
(defines, this->Makefile->GetProperty(defPropName.c_str()));
this->LocalGenerator->AppendDefines
(defines, this->Target->GetProperty(defPropName.c_str()), lang);
(defines, this->Target->GetProperty(defPropName.c_str()));
ByLanguageMap::value_type entry(l, defines);
std::string definesString;
this->LocalGenerator->JoinDefines(defines, definesString, lang);
ByLanguageMap::value_type entry(l, definesString);
i = this->DefinesByLanguage.insert(entry).first;
}
return i->second;
@ -587,14 +590,12 @@ cmMakefileTargetGenerator
}
// Add language-specific defines.
std::string defines = "$(";
defines += lang;
defines += "_DEFINES)";
std::set<std::string> defines;
// Add source-sepcific preprocessor definitions.
if(const char* compile_defs = source.GetProperty("COMPILE_DEFINITIONS"))
{
this->LocalGenerator->AppendDefines(defines, compile_defs, lang);
this->LocalGenerator->AppendDefines(defines, compile_defs);
*this->FlagFileStream << "# Custom defines: "
<< relativeObj << "_DEFINES = "
<< compile_defs << "\n"
@ -607,7 +608,7 @@ cmMakefileTargetGenerator
if(const char* config_compile_defs =
source.GetProperty(defPropName.c_str()))
{
this->LocalGenerator->AppendDefines(defines, config_compile_defs, lang);
this->LocalGenerator->AppendDefines(defines, config_compile_defs);
*this->FlagFileStream
<< "# Custom defines: "
<< relativeObj << "_DEFINES_" << configUpper
@ -676,7 +677,14 @@ cmMakefileTargetGenerator
cmLocalGenerator::SHELL);
vars.ObjectDir = objectDir.c_str();
vars.Flags = flags.c_str();
vars.Defines = defines.c_str();
std::string definesString = "$(";
definesString += lang;
definesString += "_DEFINES)";
this->LocalGenerator->JoinDefines(defines, definesString, lang);
vars.Defines = definesString.c_str();
bool lang_is_c_or_cxx = ((strcmp(lang, "C") == 0) ||
(strcmp(lang, "CXX") == 0));

View File

@ -184,46 +184,43 @@ std::string
cmNinjaTargetGenerator::
ComputeDefines(cmSourceFile *source, const std::string& language)
{
std::string defines;
std::set<std::string> defines;
// Add the export symbol definition for shared library objects.
if(const char* exportMacro = this->Target->GetExportMacro())
{
this->LocalGenerator->AppendDefines(defines, exportMacro,
language.c_str());
this->LocalGenerator->AppendDefines(defines, exportMacro);
}
// Add preprocessor definitions for this target and configuration.
this->LocalGenerator->AppendDefines
(defines,
this->Makefile->GetProperty("COMPILE_DEFINITIONS"),
language.c_str());
this->Makefile->GetProperty("COMPILE_DEFINITIONS"));
this->LocalGenerator->AppendDefines
(defines,
this->Target->GetProperty("COMPILE_DEFINITIONS"),
language.c_str());
this->Target->GetProperty("COMPILE_DEFINITIONS"));
this->LocalGenerator->AppendDefines
(defines,
source->GetProperty("COMPILE_DEFINITIONS"),
language.c_str());
source->GetProperty("COMPILE_DEFINITIONS"));
{
std::string defPropName = "COMPILE_DEFINITIONS_";
defPropName += cmSystemTools::UpperCase(this->GetConfigName());
this->LocalGenerator->AppendDefines
(defines,
this->Makefile->GetProperty(defPropName.c_str()),
language.c_str());
this->Makefile->GetProperty(defPropName.c_str()));
this->LocalGenerator->AppendDefines
(defines,
this->Target->GetProperty(defPropName.c_str()),
language.c_str());
this->Target->GetProperty(defPropName.c_str()));
this->LocalGenerator->AppendDefines
(defines,
source->GetProperty(defPropName.c_str()),
language.c_str());
source->GetProperty(defPropName.c_str()));
}
return defines;
std::string definesString;
this->LocalGenerator->JoinDefines(defines, definesString,
language.c_str());
return definesString;
}
cmNinjaDeps cmNinjaTargetGenerator::ComputeLinkDeps() const

View File

@ -218,6 +218,7 @@ if(BUILD_TESTING)
ADD_TEST_MACRO(Unset Unset)
ADD_TEST_MACRO(PolicyScope PolicyScope)
ADD_TEST_MACRO(EmptyLibrary EmptyLibrary)
ADD_TEST_MACRO(CompileDefinitions CompileDefinitions)
set_tests_properties(EmptyLibrary PROPERTIES
PASS_REGULAR_EXPRESSION "CMake Error: CMake can not determine linker language for target:test")
ADD_TEST_MACRO(CrossCompile CrossCompile)

View File

@ -0,0 +1,12 @@
cmake_minimum_required(VERSION 2.8)
project(CompileDefinitions)
add_subdirectory(add_definitions_command)
add_subdirectory(target_prop)
add_subdirectory(add_definitions_command_with_target_prop)
file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/main.cpp" "int main(int, char **) { return 0; }\n")
add_executable(CompileDefinitions "${CMAKE_CURRENT_BINARY_DIR}/main.cpp")

View File

@ -0,0 +1,7 @@
project(add_definitions_command)
add_definitions(-DCMAKE_IS_FUN -DCMAKE_IS=Fun -DCMAKE_IS_="Fun" -DCMAKE_IS_REALLY="Very Fun")
add_definitions(-DCMAKE_IS_="Fun" -DCMAKE_IS_REALLY="Very Fun" -DCMAKE_IS_FUN -DCMAKE_IS=Fun)
add_executable(add_definitions_command_executable ../main.cpp)

View File

@ -0,0 +1,14 @@
project(add_definitions_command_with_target_prop)
add_definitions(-DCMAKE_IS_FUN -DCMAKE_IS=Fun)
add_executable(add_definitions_command_with_target_prop_executable ../main.cpp)
set_target_properties(add_definitions_command_with_target_prop_executable PROPERTIES COMPILE_DEFINITIONS CMAKE_IS_="Fun")
set_property(TARGET add_definitions_command_with_target_prop_executable APPEND PROPERTY COMPILE_DEFINITIONS CMAKE_IS_REALLY="Very Fun")
add_definitions(-DCMAKE_IS_FUN)
set_property(TARGET add_definitions_command_with_target_prop_executable APPEND PROPERTY COMPILE_DEFINITIONS CMAKE_IS=Fun CMAKE_IS_="Fun")

View File

@ -0,0 +1,28 @@
#ifndef CMAKE_IS_FUN
#error Expect CMAKE_IS_FUN definition
#endif
#if CMAKE_IS != Fun
#error Expect CMAKE_IS=Fun definition
#endif
template<bool test>
struct CMakeStaticAssert;
template<>
struct CMakeStaticAssert<true> {};
static const char fun_string[] = CMAKE_IS_;
static const char very_fun_string[] = CMAKE_IS_REALLY;
enum {
StringLiteralTest1 = sizeof(CMakeStaticAssert<sizeof(CMAKE_IS_) == sizeof("Fun")>),
StringLiteralTest2 = sizeof(CMakeStaticAssert<sizeof(CMAKE_IS_REALLY) == sizeof("Very Fun")>)
};
int main(int argc, char **argv)
{
return 0;
}

View File

@ -0,0 +1,9 @@
project(target_prop)
add_executable(target_prop_executable ../main.cpp)
set_target_properties(target_prop_executable PROPERTIES CMAKE_IS_FUN -DCMAKE_IS_REALLY="Very Fun")
set_property(TARGET target_prop_executable APPEND PROPERTY COMPILE_DEFINITIONS CMAKE_IS_REALLY="Very Fun" CMAKE_IS=Fun)
set_property(TARGET target_prop_executable APPEND PROPERTY COMPILE_DEFINITIONS CMAKE_IS_FUN CMAKE_IS_="Fun")