ENH: Converted cmMakefile DefineFlags added by ADD_DEFINITIONS command into a COMPILE_DEFINITIONS directory property.

13 files changed, 222 insertions, 47 deletions

diff --git a/Source/cmAddDefinitionsCommand.h b/Source/cmAddDefinitionsCommand.h
index 7bf7116..ec35fe2 100644
--- a/Source/cmAddDefinitionsCommand.h
+++ b/Source/cmAddDefinitionsCommand.h
@@ -52,7 +52,7 @@ public:
    */
   virtual const char* GetTerseDocumentation()
     {
-    return "Adds -D define flags to the command line of C and C++ compilers.";
+    return "Adds -D define flags to the compilation of source files.";
     }
 
   /**
@@ -62,13 +62,23 @@ public:
     {
     return
       "  add_definitions(-DFOO -DBAR ...)\n"
-      "Adds flags to command line of C and C++ compilers.  "
-      "This command can be used to add any flag to a compile line, "
-      "but the -D flag is accepted most C/C++ compilers.  "
-      "Other flags may not be as portable.";
+      "Adds flags to the compiler command line for sources in the current "
+      "directory and below.  This command can be used to add any flags, "
+      "but it was originally intended to add preprocessor definitions.  "
+      "Flags beginning in -D or /D that look like preprocessor definitions "
+      "are automatically added to the COMPILE_DEFINITIONS property for "
+      "the current directory.  Definitions with non-trival values may be "
+      "left in the set of flags instead of being converted for reasons of "
+      "backwards compatibility.  See documentation of the directory, "
+      "target, and source file COMPILE_DEFINITIONS properties for details "
+      "on adding preprocessor definitions to specific scopes and "
+      "configurations."
+      ;
     }
 
   cmTypeMacro(cmAddDefinitionsCommand, cmCommand);
+private:
+  bool ParseDefinition(std::string const& def);
 };
 
 
diff --git a/Source/cmGlobalXCodeGenerator.cxx b/Source/cmGlobalXCodeGenerator.cxx
index 35bc40e..89c9377 100644
--- a/Source/cmGlobalXCodeGenerator.cxx
+++ b/Source/cmGlobalXCodeGenerator.cxx
@@ -1300,11 +1300,15 @@ void cmGlobalXCodeGenerator::CreateBuildSettings(cmTarget& target,
     // Add the export symbol definition for shared library objects.
     this->AppendDefines(ppDefs, exportMacro);
     }
+  this->AppendDefines
+    (ppDefs, this->CurrentMakefile->GetProperty("COMPILE_DEFINITIONS"));
   this->AppendDefines(ppDefs, target.GetProperty("COMPILE_DEFINITIONS"));
   if(configName)
     {
     std::string defVarName = "COMPILE_DEFINITIONS_";
     defVarName += cmSystemTools::UpperCase(configName);
+    this->AppendDefines
+      (ppDefs, this->CurrentMakefile->GetProperty(defVarName.c_str()));
     this->AppendDefines(ppDefs, target.GetProperty(defVarName.c_str()));
     }
   buildSettings->AddAttribute
diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index 3ac703d..23179dc 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -1179,7 +1179,6 @@ const char* cmLocalGenerator::GetIncludeFlags(const char* lang)
     flags[flags.size()-1] = ' ';
     }
   std::string defineFlags = this->Makefile->GetDefineFlags();
-  this->FixDefineFlags(defineFlags, lang);
   flags += defineFlags;
   this->LanguageToIncludeFlags[lang] = flags;
 
@@ -1190,40 +1189,6 @@ const char* cmLocalGenerator::GetIncludeFlags(const char* lang)
 }
 
 //----------------------------------------------------------------------------
-void cmLocalGenerator::FixDefineFlags(std::string& flags, 
-                                      const char* lang)
-{
-  std::string defineFlagVar = "CMAKE_";
-  defineFlagVar += lang;
-  defineFlagVar += "_DEFINE_FLAG";
-  std::string defineFlag = 
-    this->Makefile->GetSafeDefinition(defineFlagVar.c_str());
-  if(defineFlag.size() == 0)
-    {
-    return;
-    }
-  std::vector<std::string> args;
-  cmSystemTools::ParseWindowsCommandLine(flags.c_str(), args);
-  std::string fixedFlags;
-  const char* sep = 0;
-  for(std::vector<std::string>::iterator i = args.begin();
-      i != args.end(); ++i)
-    {
-    if(sep)
-      {
-      fixedFlags += sep;
-      }
-    else
-      {
-      sep = " ";
-      }
-    cmSystemTools::ReplaceString(*i, "-D", defineFlag.c_str());
-    fixedFlags += *i;
-    }
-  flags = fixedFlags;
-}
-
-//----------------------------------------------------------------------------
 void cmLocalGenerator::GetIncludeDirectories(std::vector<std::string>& dirs,
                                              bool filter_system_dirs)
 {
diff --git a/Source/cmLocalGenerator.h b/Source/cmLocalGenerator.h
index ce0d0d6..721080e 100644
--- a/Source/cmLocalGenerator.h
+++ b/Source/cmLocalGenerator.h
@@ -278,7 +278,6 @@ public:
                                   unsigned int minor,
                                   unsigned int patch = 0xFFu);
 protected:
-  void FixDefineFlags(std::string& defineFlags, const char* lang);
   /** Construct a comment for a custom command.  */
   std::string ConstructComment(const cmCustomCommand& cc,
                                const char* default_comment = "");
diff --git a/Source/cmLocalVisualStudio6Generator.cxx b/Source/cmLocalVisualStudio6Generator.cxx
index a79f8a7..2cfcf70 100644
--- a/Source/cmLocalVisualStudio6Generator.cxx
+++ b/Source/cmLocalVisualStudio6Generator.cxx
@@ -1503,16 +1503,30 @@ void cmLocalVisualStudio6Generator
       }
 
     // Add per-target and per-configuration preprocessor definitions.
+    this->AppendDefines
+      (flags, this->Makefile->GetProperty("COMPILE_DEFINITIONS"), 0);
     this->AppendDefines(flags, target.GetProperty("COMPILE_DEFINITIONS"), 0);
+    this->AppendDefines
+      (flagsDebug,
+       this->Makefile->GetProperty("COMPILE_DEFINITIONS_DEBUG"), 0);
     this->AppendDefines(flagsDebug,
                         target.GetProperty("COMPILE_DEFINITIONS_DEBUG"), 0);
+    this->AppendDefines
+      (flagsRelease,
+       this->Makefile->GetProperty("COMPILE_DEFINITIONS_RELEASE"), 0);
     this->AppendDefines(flagsRelease,
                         target.GetProperty("COMPILE_DEFINITIONS_RELEASE"), 0);
     this->AppendDefines
       (flagsMinSize,
+       this->Makefile->GetProperty("COMPILE_DEFINITIONS_MINSIZEREL"), 0);
+    this->AppendDefines
+      (flagsMinSize,
        target.GetProperty("COMPILE_DEFINITIONS_MINSIZEREL"), 0);
     this->AppendDefines
       (flagsDebugRel,
+       this->Makefile->GetProperty("COMPILE_DEFINITIONS_RELWITHDEBINFO"), 0);
+    this->AppendDefines
+      (flagsDebugRel,
        target.GetProperty("COMPILE_DEFINITIONS_RELWITHDEBINFO"), 0);
 
     // The template files have CXX FLAGS in them, that need to be replaced.
diff --git a/Source/cmLocalVisualStudio7Generator.cxx b/Source/cmLocalVisualStudio7Generator.cxx
index 2480622..f9ebb85 100644
--- a/Source/cmLocalVisualStudio7Generator.cxx
+++ b/Source/cmLocalVisualStudio7Generator.cxx
@@ -532,7 +532,10 @@ void cmLocalVisualStudio7Generator::WriteConfiguration(std::ostream& fout,
   targetOptions.FixExceptionHandlingDefault();
   targetOptions.Parse(flags.c_str());
   targetOptions.Parse(defineFlags.c_str());
+  targetOptions.AddDefines
+    (this->Makefile->GetProperty("COMPILE_DEFINITIONS"));
   targetOptions.AddDefines(target.GetProperty("COMPILE_DEFINITIONS"));
+  targetOptions.AddDefines(this->Makefile->GetProperty(defPropName.c_str()));
   targetOptions.AddDefines(target.GetProperty(defPropName.c_str()));
   targetOptions.SetVerboseMakefile(
     this->Makefile->IsOn("CMAKE_VERBOSE_MAKEFILE"));
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 104d007..5a3be94 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -877,6 +877,12 @@ void cmMakefile::AddDefineFlag(const char* flag)
     return;
     }
 
+  // If this is really a definition, update COMPILE_DEFINITIONS.
+  if(this->ParseDefineFlag(flag, false))
+    {
+    return;
+    }
+
   // remove any \n\r
   std::string ret = flag;
   std::string::size_type pos = 0;
@@ -906,6 +912,12 @@ void cmMakefile::RemoveDefineFlag(const char* flag)
     return;
     }
 
+  // If this is really a definition, update COMPILE_DEFINITIONS.
+  if(this->ParseDefineFlag(flag, true))
+    {
+    return;
+    }
+
   // Remove all instances of the flag that are surrounded by
   // whitespace or the beginning/end of the string.
   for(std::string::size_type lpos = this->DefineFlags.find(flag, 0);
@@ -924,6 +936,67 @@ void cmMakefile::RemoveDefineFlag(const char* flag)
     }
 }
 
+bool cmMakefile::ParseDefineFlag(std::string const& def, bool remove)
+{
+  // Create a regular expression to match valid definitions.
+  // Definitions with non-trivial values must not be matched because
+  // escaping them could break compatibility with escapes added by
+  // users.
+  static cmsys::RegularExpression
+    regex("^[-/]D[A-Za-z_][A-Za-z0-9_]*(=[A-Za-z0-9_.]+)?$");
+
+  // Make sure the definition matches.
+  if(!regex.find(def.c_str()))
+    {
+    return false;
+    }
+
+  // VS6 IDE does not support definitions with values.
+  if((strcmp(this->LocalGenerator->GetGlobalGenerator()->GetName(),
+             "Visual Studio 6") == 0) &&
+     (def.find("=") != def.npos))
+    {
+    return false;
+    }
+
+  // Get the definition part after the flag.
+  const char* define = def.c_str() + 2;
+
+  if(remove)
+    {
+    if(const char* cdefs = this->GetProperty("COMPILE_DEFINITIONS"))
+      {
+      // Expand the list.
+      std::vector<std::string> defs;
+      cmSystemTools::ExpandListArgument(cdefs, defs);
+
+      // Recompose the list without the definition.
+      std::string ndefs;
+      const char* sep = "";
+      for(std::vector<std::string>::const_iterator di = defs.begin();
+          di != defs.end(); ++di)
+        {
+        if(*di != define)
+          {
+          ndefs += sep;
+          sep = ";";
+          ndefs += *di;
+          }
+        }
+
+      // Store the new list.
+      this->SetProperty("COMPILE_DEFINITIONS", ndefs.c_str());
+      }
+    }
+  else
+    {
+    // Append the definition to the directory property.
+    this->AppendProperty("COMPILE_DEFINITIONS", define);
+    }
+
+  return true;
+}
+
 void cmMakefile::AddLinkLibrary(const char* lib,
                                 cmTarget::LinkLibraryType llt)
 {
@@ -2960,6 +3033,35 @@ void cmMakefile::DefineProperties(cmake *cm)
      "included and processed when ctest is run on the directory.");
 
   cm->DefineProperty
+    ("COMPILE_DEFINITIONS", cmProperty::DIRECTORY,
+     "Preprocessor definitions for compiling a directory's sources.",
+     "The COMPILE_DEFINITIONS property may be set to a list of preprocessor "
+     "definitions using the syntax VAR or VAR=value.  Function-style "
+     "definitions are not supported.  CMake will automatically escape "
+     "the value correctly for the native build system (note that CMake "
+     "language syntax may require escapes to specify some values).  "
+     "This property may be set on a per-configuration basis using the name "
+     "COMPILE_DEFINITIONS_<CONFIG> where <CONFIG> is an upper-case name "
+     "(ex. \"COMPILE_DEFINITIONS_DEBUG\").\n"
+     "CMake will automatically drop some definitions that "
+     "are not supported by the native build tool.  "
+     "The VS6 IDE does not support definitions with values "
+     "(but NMake does).\n"
+     "Dislaimer: Most native build tools have poor support for escaping "
+     "certain values.  CMake has work-arounds for many cases but some "
+     "values may just not be possible to pass correctly.  If a value "
+     "does not seem to be escaped correctly, do not attempt to "
+     "work-around the problem by adding escape sequences to the value.  "
+     "Your work-around may break in a future version of CMake that "
+     "has improved escape support.  Instead consider defining the macro "
+     "in a (configured) header file.  Then report the limitation.");
+
+  cm->DefineProperty
+    ("COMPILE_DEFINITIONS_<CONFIG>", cmProperty::DIRECTORY,
+     "Per-configuration preprocessor definitions in a directory.",
+     "This is the configuration-specific version of COMPILE_DEFINITIONS.");
+
+  cm->DefineProperty
     ("EXCLUDE_FROM_ALL", cmProperty::DIRECTORY,
      "Exclude the directory from the all target of its parent.",
      "A property on a directory that indicates if its targets are excluded "
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 989cd15..201a507 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -801,6 +801,8 @@ protected:
 private:
   void Initialize();
 
+  bool ParseDefineFlag(std::string const& definition, bool remove);
+
   void ReadSources(std::ifstream& fin, bool t);
   friend class cmMakeDepend;    // make depend needs direct access
                                 // to the Sources array 
diff --git a/Source/cmMakefileTargetGenerator.cxx b/Source/cmMakefileTargetGenerator.cxx
index ce1442f..37bd5f9 100644
--- a/Source/cmMakefileTargetGenerator.cxx
+++ b/Source/cmMakefileTargetGenerator.cxx
@@ -267,11 +267,15 @@ void cmMakefileTargetGenerator::WriteTargetLanguageFlags()
 
     // Add preprocessor definitions for this target and configuration.
     this->LocalGenerator->AppendDefines
+      (defines, this->Makefile->GetProperty("COMPILE_DEFINITIONS"), lang);
+    this->LocalGenerator->AppendDefines
       (defines, this->Target->GetProperty("COMPILE_DEFINITIONS"), lang);
     std::string defPropName = "COMPILE_DEFINITIONS_";
     defPropName +=
       cmSystemTools::UpperCase(this->LocalGenerator->ConfigurationName);
     this->LocalGenerator->AppendDefines
+      (defines, this->Makefile->GetProperty(defPropName.c_str()), lang);
+    this->LocalGenerator->AppendDefines
       (defines, this->Target->GetProperty(defPropName.c_str()), lang);
 
     // Add language-specific flags.
diff --git a/Source/cmRemoveDefinitionsCommand.h b/Source/cmRemoveDefinitionsCommand.h
index 2444ab0..8ee5086 100644
--- a/Source/cmRemoveDefinitionsCommand.h
+++ b/Source/cmRemoveDefinitionsCommand.h
@@ -53,8 +53,7 @@ public:
    */
   virtual const char* GetTerseDocumentation() 
     {
-    return 
-      "Removes -D define flags to the command line of C and C++ compilers.";
+    return "Removes -D define flags added by add_definitions.";
     }
   
   /**
@@ -64,10 +63,8 @@ public:
     {
     return
       "  remove_definitions(-DFOO -DBAR ...)\n"
-      "Removes flags from command line of C and C++ compilers.  "
-      "This command can be used to remove any flag from a compile line, "
-      "but the -D flag is accepted by most C/C++ compilers.  "
-      "Other flags may not be as portable.";
+      "Removes flags (added by add_definitions) from the compiler command "
+      "line for sources in the current directory and below.";
     }
   
   cmTypeMacro(cmRemoveDefinitionsCommand, cmCommand);
diff --git a/Tests/Preprocess/CMakeLists.txt b/Tests/Preprocess/CMakeLists.txt
index 58bddbd..ee2e355 100644
--- a/Tests/Preprocess/CMakeLists.txt
+++ b/Tests/Preprocess/CMakeLists.txt
@@ -144,6 +144,17 @@ endif("${CMAKE_GENERATOR}" MATCHES "Visual Studio 6")
 # Test old-style definitions.
 add_definitions(-DOLD_DEF -DOLD_EXPR=2)
 
+# Make sure old-style definitions are converted to directory property.
+if(PREPROCESS_VS6)
+  set(OLD_DEFS_EXPECTED "OLD_DEF")
+else(PREPROCESS_VS6)
+  set(OLD_DEFS_EXPECTED "OLD_DEF;OLD_EXPR=2")
+endif(PREPROCESS_VS6)
+get_property(OLD_DEFS DIRECTORY PROPERTY COMPILE_DEFINITIONS)
+if(NOT "${OLD_DEFS}" STREQUAL "${OLD_DEFS_EXPECTED}")
+  message(SEND_ERROR "add_definitions not converted to directory property!")
+endif(NOT "${OLD_DEFS}" STREQUAL "${OLD_DEFS_EXPECTED}")
+
 add_executable(Preprocess preprocess.c preprocess${VS6}.cxx)
 
 set(FILE_PATH "${Preprocess_SOURCE_DIR}/file_def.h")
@@ -152,6 +163,10 @@ set(TARGET_PATH "${Preprocess_SOURCE_DIR}/target_def.h")
 # Set some definition properties.
 foreach(c "" "_DEBUG" "_RELEASE")
   set_property(
+    DIRECTORY .
+    APPEND PROPERTY COMPILE_DEFINITIONS${c} "DIRECTORY_DEF${c}"
+    )
+  set_property(
     TARGET Preprocess
     PROPERTY COMPILE_DEFINITIONS${c} "TARGET_DEF${c}"
     )
diff --git a/Tests/Preprocess/preprocess.c b/Tests/Preprocess/preprocess.c
index baa18df..5dd9003 100644
--- a/Tests/Preprocess/preprocess.c
+++ b/Tests/Preprocess/preprocess.c
@@ -52,6 +52,12 @@ int check_defines_C(void)
   result = 0;
   }
 # endif
+# ifdef DIRECTORY_DEF_DEBUG
+  {
+  fprintf(stderr, "DIRECTORY_DEF_DEBUG should not be defined in C\n");
+  result = 0;
+  }
+# endif
 # ifndef FILE_DEF_RELEASE
 #  ifndef PREPROCESS_XCODE
   {
@@ -66,6 +72,12 @@ int check_defines_C(void)
   result = 0;
   }
 # endif
+# ifndef DIRECTORY_DEF_RELEASE
+  {
+  fprintf(stderr, "DIRECTORY_DEF_RELEASE should be defined in C\n");
+  result = 0;
+  }
+# endif
 #endif
 #ifdef PREPROCESS_DEBUG
 # ifndef FILE_DEF_DEBUG
@@ -82,6 +94,12 @@ int check_defines_C(void)
   result = 0;
   }
 # endif
+# ifndef DIRECTORY_DEF_DEBUG
+  {
+  fprintf(stderr, "DIRECTORY_DEF_DEBUG should be defined in C\n");
+  result = 0;
+  }
+# endif
 # ifdef FILE_DEF_RELEASE
   {
   fprintf(stderr, "FILE_DEF_RELEASE should not be defined in C\n");
@@ -94,6 +112,12 @@ int check_defines_C(void)
   result = 0;
   }
 # endif
+# ifdef DIRECTORY_DEF_RELEASE
+  {
+  fprintf(stderr, "DIRECTORY_DEF_RELEASE should not be defined in C\n");
+  result = 0;
+  }
+# endif
 #endif
 #if defined(FILE_DEF_DEBUG) || defined(TARGET_DEF_DEBUG)
 # if !defined(FILE_DEF_DEBUG) || !defined(TARGET_DEF_DEBUG)
@@ -153,6 +177,12 @@ int check_defines_C(void)
   result = 0;
   }
 #endif
+#ifndef DIRECTORY_DEF
+  {
+  fprintf(stderr, "DIRECTORY_DEF not defined in C\n");
+  result = 0;
+  }
+#endif
 #ifndef OLD_DEF
   {
   fprintf(stderr, "OLD_DEF not defined in C\n");
diff --git a/Tests/Preprocess/preprocess.cxx b/Tests/Preprocess/preprocess.cxx
index 07b7183..628521f 100644
--- a/Tests/Preprocess/preprocess.cxx
+++ b/Tests/Preprocess/preprocess.cxx
@@ -54,6 +54,12 @@ int check_defines_CXX()
   result = 0;
   }
 # endif
+# ifdef DIRECTORY_DEF_DEBUG
+  {
+  fprintf(stderr, "DIRECTORY_DEF_DEBUG should not be defined in CXX\n");
+  result = 0;
+  }
+# endif
 # ifndef FILE_DEF_RELEASE
 #  ifndef PREPROCESS_XCODE
   {
@@ -68,6 +74,12 @@ int check_defines_CXX()
   result = 0;
   }
 # endif
+# ifndef DIRECTORY_DEF_RELEASE
+  {
+  fprintf(stderr, "DIRECTORY_DEF_RELEASE should be defined in CXX\n");
+  result = 0;
+  }
+# endif
 #endif
 #ifdef PREPROCESS_DEBUG
 # ifndef FILE_DEF_DEBUG
@@ -84,6 +96,12 @@ int check_defines_CXX()
   result = 0;
   }
 # endif
+# ifndef DIRECTORY_DEF_DEBUG
+  {
+  fprintf(stderr, "DIRECTORY_DEF_DEBUG should be defined in CXX\n");
+  result = 0;
+  }
+# endif
 # ifdef FILE_DEF_RELEASE
   {
   fprintf(stderr, "FILE_DEF_RELEASE should not be defined in CXX\n");
@@ -96,6 +114,12 @@ int check_defines_CXX()
   result = 0;
   }
 # endif
+# ifdef DIRECTORY_DEF_RELEASE
+  {
+  fprintf(stderr, "DIRECTORY_DEF_RELEASE should not be defined in CXX\n");
+  result = 0;
+  }
+# endif
 #endif
 #if defined(FILE_DEF_DEBUG) || defined(TARGET_DEF_DEBUG)
 # if !defined(FILE_DEF_DEBUG) || !defined(TARGET_DEF_DEBUG)
@@ -155,6 +179,12 @@ int check_defines_CXX()
   result = 0;
   }
 #endif
+#ifndef DIRECTORY_DEF
+  {
+  fprintf(stderr, "DIRECTORY_DEF not defined in CXX\n");
+  result = 0;
+  }
+#endif
 #ifndef OLD_DEF
   {
   fprintf(stderr, "OLD_DEF not defined in CXX\n");