Notes |
|
(0015762)
|
Twylite
|
2009-03-23 11:57
|
|
Severity is quite possibly wrong - I had intended to use "minor".
May be related to issue 0005099. Not checked with MSVC7+. |
|
|
(0015766)
|
Brad King
|
2009-03-23 14:53
|
|
As of CMake 2.6.3 all the generators support values with proper escaping except VS 6. This was implemented as part of creating the COMPILE_DEFINITIONS directory/target/source-file properties. See policy CMP0005 documentation:
http://www.cmake.org/cmake/help/cmake2.6docs.html#policy:CMP0005 [^]
When I worked on that feature I mistakenly concluded after some testing that VS6 does not support preprocessor values very well. If you try to put a value in with COMPILE_DEFINITIONS and the VS6 generator you will get a warning. For compatibility CMake remains silent in this case for ADD_DEFINITIONS and just passes through the user value.
After further investigation I see that VS6 supports values that do not contain spaces. I've committed changes that allow all macros except those with spaces in their value:
ENH: Support preprocessor def values in VS6
/cvsroot/CMake/CMake/Source/cmLocalVisualStudio6Generator.cxx,v <-- Source/cmLocalVisualStudio6Generator.cxx
new revision: 1.151; previous revision: 1.150
/cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- Source/cmMakefile.cxx
new revision: 1.507; previous revision: 1.506
/cvsroot/CMake/CMake/Source/cmSourceFile.cxx,v <-- Source/cmSourceFile.cxx
new revision: 1.56; previous revision: 1.55
/cvsroot/CMake/CMake/Source/cmTarget.cxx,v <-- Source/cmTarget.cxx
new revision: 1.237; previous revision: 1.236
/cvsroot/CMake/CMake/Tests/Preprocess/CMakeLists.txt,v <-- Tests/Preprocess/CMakeLists.txt
new revision: 1.11; previous revision: 1.10
/cvsroot/CMake/CMake/Tests/Preprocess/preprocess.c,v <-- Tests/Preprocess/preprocess.c
new revision: 1.3; previous revision: 1.2
/cvsroot/CMake/CMake/Tests/Preprocess/preprocess.cxx,v <-- Tests/Preprocess/preprocess.cxx
new revision: 1.3; previous revision: 1.2 |
|
|
(0015768)
|
Brad King
|
2009-03-23 15:05
|
|
I've scheduled the fix for inclusion in the 2.6 release branch. |
|
|
(0015769)
|
Twylite
|
2009-03-23 17:24
|
|
Wow - seriously impressed with the response time! :)
This has ended up being a rather long response. The summary:
1. MSVC6 DSPs handle defines with values with spaces just fine. The general form is -D"NAME=value" or -DNAME="value", where value has C-style backslash quoting (e.g. \" for a literal double-quote)
2. The use-case is for the substitution of a path via a compile-time defines, and the default path is under C:\Program Files\, so support for spaces is kindof important to me ;)
3. COMPILE_DEFINITIONS PATH="c:\\\\program files\\\\my app\\\\resources"
should become
# ADD CPP /D PATH="\"c:\\program files\\my app\\resources\""
or
# ADD CPP -D"PATH=\"c:\\program files\\my app\\resources\""
to behave the same as NMake and MSVC2008. The latter is identical to the substitution make for NMake (or at least how it ends up on the cl command line via the NMake generator), and conceptually the same as the approach used in MSVC2008.
---
I think I underestimated the complexity of this problem, so I'm going to have to get more thorough.
In the Tcl sources (specifically tclPkgConfig.c) there is an array that is initialised from compile-time defines; here's a snippet:
static Tcl_Config const cfg[] = {
{"libdir,runtime", CFG_RUNTIME_LIBDIR},
{NULL, NULL}
};
The normal Tcl build process uses automake/autoconf, which generates appropriate defines for the Unix environment. On Windows there is a hand-crafted Makefile for NMake that has appropriate defines for WIN32.
The default for CFG_RUNTIME_LIBDIR is "C:\\Program Files\\Tcl\\lib" (that is the literal that needs to be substituted in the C source, and includes C quoting of the backslashes).
So I need to use a compile-time define to set up certain constants, some of which are paths.
In an isolated experiment I could reduce this to e.g.
printf( "c:\\program files\\my app\\resources" );
needs to be:
printf( PATH );
so I need to define:
PATH="c:\\program files\\my app\\resources"
but correctly quoted for the command line, tool or project file.
I'm going to update my bug.c to illustrate the problem:
---
#include <stdio.h>
void main()
{
printf( PATH );
}
---
And I'll make a corresponding update to the CMakeLists.txt:
---
cmake_minimum_required (VERSION 2.6)
project (bug C)
cmake_policy( SET CMP0005 NEW )
add_executable( bug bug.c )
#add_definitions( -DPATH="c:\\program files\\my app\\resources" )
set_property( TARGET bug PROPERTY COMPILE_DEFINITIONS PATH="c:\\program files\\my app\\resources" )
---
Note on CMake quoting:
-
Using the more thoroughly quoted
COMPILE_DEFINITIONS "PATH=\"c:\\program files\\my app\\resources\""
is no different to the CMakeLists.txt presented above.
-
Note on VS6 define support:
-
In the VS6 IDE I can go to Project Settings, C++, Preprocessor
Definitions and add:
PATH="\"c:\\program files\\my app\\resources\""
Which appears in the .DSP as:
# ADD CPP /D PATH="\"c:\\program files\\my app\\resources\""
There general format for a define in a VS6 DSP seems to be
NAME="value"
where value can contain C-like escape sequences
The quotes can go around the entire define, so the general form
can also be described as
"NAME=value"
which appears in the DSP as:
# ADD CPP -D"PATH=\"c:\\program files\\my app\\resources\""
-
--- RESULTS WITH CMAKE 2.6.3 ---
Results for CMake 2.6.3, CMP0005 OLD, NMakefile
cl ... -DPATH="c:\program files\my app\resources"
bug.c(5) : error C2065: 'c' : undeclared identifier
Results for CMake 2.6.3, CMP0005 NEW, NMakefile
cl ... -D"PATH=\"c:\program files\my app\resources\""
bug.c(5) : warning C4129: 'p' : unrecognized character escape sequence
I get the same results for both the add_definitions() and set_property() approaches.
It is worth noting that the CMP0005 NEW escapes the double-quotes, but doesn't escape the backslash.
The intended outcome is
cl ... -D"PATH=\"c:\\program files\\my app\\resources\""
To get that outcome with CMake 2.6.3 I will have to add extra escapes for the backslashes in the COMPILE_DEFINITIONS, which seems like the wrong thing to do? (UPDATE: I tried it below, and it seems to be the right thing to do ...)
For the Visual Studio 6 generator only add_definitions() can be used (COMPILE_DEFINITIONS gives "WARNING: The VS6 IDE does not support preprocessor definitions with values." as expected)
Results for CMake 2.6.3, Visual Studio 6
# ADD CPP -DPATH="c:\program files\my app\resources"
The results do not differ for CMP0005 OLD / NEW.
The intended outcome is
# ADD CPP /D PATH="\"c:\\program files\\my app\\resources\""
Results for CMake 2.6.3, CMP0005 NEW, Visual Studio 9 2008
"PATH=\"c:\program files\my app\resources\""
bug.c(5) : warning C4129: 'p' : unrecognized character escape sequence
Okay, the behaviour here is consistent with the NMake generator, but a little surprising (again, double-quotes are correctly quoted, but backslashes go into the .proj verbatim and end up being interpreted as escape characters).
--- TAKE 2: EXTRA QUOTING OF BACKSLASHES IN THE CMAKELISTS ---
I'll update my CMakeLists.txt and try again:
---
cmake_minimum_required (VERSION 2.6)
project (bug C)
cmake_policy( SET CMP0005 NEW )
add_executable( bug bug.c )
set_property( TARGET bug PROPERTY COMPILE_DEFINITIONS PATH="c:\\\\program files\\\\my app\\\\resources" )
---
This time NMake and VS2008 build correctly.
nmake: cl ... -D"PATH=\"c:\\program files\\my app\\resources\""
.proj: "PATH=\"c:\\program files\\my app\\resources\""
MSVC6 (using add_definitions()) still has problems, but is closer to the required value:
# ADD CPP -DPATH="c:\\program files\\my app\\resources"
bug.c(5) : error C2065: 'c' : undeclared identifier
what it should be:
# ADD CPP /D PATH="\"c:\\program files\\my app\\resources\""
--- RESULTS WITH CMAKE 2.7.20090323 ---
Using the new CMakeLists.txt with
COMPILE_DEFINITIONS PATH="c:\\\\program files\\\\my app\\\\resources"
resulted in warnings (as expected: The VS6 IDE does not support preprocessor definition values with spaces)
Using add_definitions() instead produced the same (buggy) result as CMake 2.6.3.
Tests with a value that did not contain spaces
set_property( TARGET bug APPEND PROPERTY COMPILE_DEFINITIONS PATH="teststr" )
were successful - the value was correctly quoted in the .dsp.
--- CONCLUSION ---
To make the VS6 generator behave the same as NMake and VS2008, simply add quotes around the name=value (as with the other 2 generators), e.g.
-D"NAME=VALUE" |
|
|
(0016043)
|
Brad King
|
2009-04-16 15:29
|
|
All generators know how to escape backslashes correctly for COMPILE_DEFINITIONS. The reason you need to put four in instead of two in the CMakeLists.txt file is because the CMake language processor evaluates one level of escapes, so the property value ends up with only one backslash for every pair written.
The only other issue is spaces in VS6 IDE. I'll investigate further. |
|
|
(0016044)
|
Brad King
|
2009-04-16 15:52
|
|
Okay, I create a simple project in VS6. Then I edit the settings, go to the list of preprocessor definitions, and add "STR=\"str with spaces\"". It builds as expected. I save the project, and in the .dsp file I see
/D "STR=\"str with spaces\""
as you suggest. Next, I exit VS6, restart it, and then load the project. The definition has disappeared from the list, and the command line has
/D /other /options "STR=\" str with spaces\"" /c
so the compiler looks for a source file with a strange name (note also the extra space before "str"). Clearly something goes wrong in the project file parser. |
|
|
(0016049)
|
Twylite
|
2009-04-17 05:49
|
|
I can reproduce this. It's rather strange that I didn't experience this in my other development environment -- I'll investigate the discrepancy, but I suspect lucky chance ;(
To be accurate, the problem isn't a string with spaces, but a string with (quoted) double-quotes.
I'll investigate whether MSVC6 can in fact handle this case, and report back. |
|
|
(0016081)
|
Brad King
|
2009-04-20 08:47
(edited on: 2009-04-20 08:48) |
|
Okay, I look forward to the results of your investigation.
|
|
|
(0016082)
|
Twylite
|
2009-04-20 09:00
|
|
What I've seen so far is that it's definitely the .DSP parser that's causing the problem, and that embedded double-quotes are an issue.
STR="str with spaces" is consistently fine.
STR=\"str with spaces\" seems fine, but I haven't looked at that much
STR="\"str with spaces\"" corrupts in all cases except where the define is the last thing on the #ADD CPP line other than the trailing /c. In that case the .DSP parser appears to pick up '/D STR="\' as the define, and then picks up some extra stuff '"str with spaces\""' which is appends to the command line to cl.
I have not yet found a way to quote the double-quotes in the value. Nothing that actually requires double-quotes (like \") appears to be compatible with the .DSP parser. I haven't identified any other escape mechanism in use by the parser, and there is no trigraph for a double-quote ;/
I should be able to come back with a conclusive answer by Thursday evening (GMT+2). |
|
|
(0016178)
|
Twylite
|
2009-04-24 09:28
|
|
Or Friday afternoon, as the case may be.
I've tested a bunch of scenarios using the following round-trip:
(1) Add define(s) under Project -> Settings -> C/C++ (Tab) -> Category: Preprocessor (dropdown) -> Preprocessor definitions
(2) Compile (rebuild all), test, File -> Save All
(3) Exit & restart MSVC6
(4) Compile (rebuild all), test
(5) Remove definitions from project settings, save, exit
(6) Add definitions manually into DSP
(7) Start MSVC6, compile (rebuild all), test
The program source was:
--
#include <stdio.h>
#define __QUOTECLEAR(str) #str
#define QUOTE(str) __QUOTECLEAR(str)
int main(int argc, char* argv[])
{
printf("Hello World!\n");
printf( QUOTE(KEY) );
printf("\n");
return 0;
}
--
Note that the construction of the program to use the string pasting functionality of the preprocessor was to allow the program to compile in run in situations where a literal double-quote could not be introduced using /D in the project setting (as it turns out, all cases).
What works:
(A) /D KEY=val
(B) /D KEY="val"
(C) /D KEY=\"val\" -> output includes ""
(D) /D KEY="val with spaces"
(E) /D KEY="val \n with \t escapes and ??< trigraphs"
(F) Any combination of multiple defines quoted as above (e.g. I defined KEY, KEY2, KEY3 and adjusted the program to display all 3)
What doesn't work:
(G) /D KEY=\"val with spaces\"
(H) /D KEY="\"val\""
(I) /D KEY="\"val with spaces\""
(J) Any attempt whatsoever to include a literal '"' in the value of a define
* (G), (H) and (I) all work in the special case where the .DSP is modified such that there is only one define of this form, and it comes after all options on the #ADD CPP line other than the trailing /c. In all other cases the "Project Options" (i.e. the cl command line as displayed in Project Settings -> C/C++ -> General) is mangled by the presence of additional double-quotes.
In most cases that use double-quotes, the first quote can come before the key name or directly after the =. I didn't test all these cases, and this isn't how MSVC saves its settings.
My conclusion:
- Using the form '/D KEY="value"' is able to support values with or without spaces.
- There is no way to introduce a literal '"' into the value. Any attempt at quoting breaks the .DSP parser.
- The form '/D KEY=\"value\"' only works for a value without spaces.
Recommendation: When writing defines into an MSVC6 .DSP use the form '/D KEY="value"'. Allow spaces, backslashes, etc. but reject double-quotes. It may be worth indicating in the relevant documentation or policy help that this is an unresolvable problem, with a reference to this issue. |
|
|
(0016179)
|
Twylite
|
2009-04-24 09:33
|
|
Additional note on the consequences of this problem with .dsp parsing:
If you need to support MSVC6, and you need to pass a string as a define on the command line (via /D) such that it will become a literal char*, then either:
(a) The string must have no spaces, and you can use /D KEY=\"val\"
OR
(b) The string may have spaces, you must use /D KEY="val with spaces", and you must use the preprocessor trick in the previous note to stringify the value. |
|
|
(0016180)
|
Brad King
|
2009-04-24 11:21
|
|
Thanks for the analysis. I've converted the definition command-line generator to use NAME="value" instead of "NAME=value" (since the NAME= part should never need quoting anyway). This enables many more values, so I ran some more experiments with values containing every character on the US keyboard.
I conclude that almost any value is valid if it does not have any spaces in combination with '"', '$', or ';'. All three characters require double quotes to escape them, which confuses the parser. I've committed changes that make the value check reject only these invalid combinations.
ENH: Support more preprocessor values in VS6
/cvsroot/CMake/CMake/Source/cmLocalGenerator.cxx,v <-- Source/cmLocalGenerator.cxx
new revision: 1.301; previous revision: 1.300
/cvsroot/CMake/CMake/Source/cmLocalVisualStudio6Generator.cxx,v <-- Source/cmLocalVisualStudio6Generator.cxx
new revision: 1.152; previous revision: 1.151
/cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- Source/cmMakefile.cxx
new revision: 1.508; previous revision: 1.507
/cvsroot/CMake/CMake/Tests/Preprocess/CMakeLists.txt,v <-- Tests/Preprocess/CMakeLists.txt
new revision: 1.12; previous revision: 1.11
ENH: Test spaces in non-string preprocessor values
/cvsroot/CMake/CMake/Tests/Preprocess/CMakeLists.txt,v <-- Tests/Preprocess/CMakeLists.txt
new revision: 1.13; previous revision: 1.12 |
|
|
(0016181)
|
Twylite
|
2009-04-24 11:32
|
|
Great.
Just to confirm -- is '/D KEY="val $with ;spaces"' a valid combination? I've just done a round-trip test with that and '/D KEY="val $ with ; spaces"' and there are no apparent problems. |
|
|
(0016183)
|
Brad King
|
2009-04-24 11:38
|
|
No. While VS6 does support them I would have to introduce several special code paths in CMake to support them. Currently all escaping goes through one path, and that path writes $ as "$" and ; as ";" because this is necessary elsewhere in the IDE files. It's not worth the effort or maintenance for the corner-case. |
|
|
(0016185)
|
Twylite
|
2009-04-24 12:20
|
|
Ah :) Okay, not a MSVC6 issue then - that's the part I was wondering about.
Sounds reasonable not to support that particular corner case then.
I'll try to find the time this weekend to work with the new revision; otherwise I consider this issue to be resolved. Thanks :) |
|
|
(0016233)
|
Brad King
|
2009-04-28 17:34
|
|
CMake 2.6.4 was just released. It has the first fix I did (allowing values without spaces). Unfortunately the second fix (allowing almost all values) was too late in the release cycle to make it. I've scheduled the second part for 2.6.5. |
|