View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0008226CMakeCMakepublic2008-12-04 17:502015-03-02 08:57
ReporterBill Hoffman 
Assigned ToBill Hoffman 
PrioritynormalSeverityfeatureReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake-2-6 
Target VersionFixed in VersionCMake 3.1 
Summary0008226: if expansion of quoted arguments as variables should be deprecated
DescriptionIf you have if("foo" MATCHES "foo" ) cmake will look for a variable foo and if there happens to be found it will replace the string. This causes unexpected things to happen.

Brad suggests:

We can do it with a policy, but it won't be too bad if we do it right.

I doubt too much code writes

  if("${foo}")

where the value of foo is another variable name to be tested.
Similarly, I doubt much code writes

  if("foo")

where it intends the variable 'foo' to be tested. The ambiguous cases
are mostly when no quotes are used.

Unlike other commands, the IF command is invoked directly with the
std::vector<cmListFileArgument> list of arguments. Each one has a
boolean indicating whether it was actually quoted or not. We can make
the policy say that if real quotes were given by the caller then to
treat it as a string and never a variable. Without quotes, the old
behavior can stay.
TagsNo tags attached.
Attached Files

- Relationships Relation Graph ] Dependency Graph ]
related to 0008823closedBill Hoffman if(variable STREQUAL string) can treak LHS as a variable name 

-  Notes
(0014293)
mwoehlke (developer)
2008-12-04 18:36

Just my own $0.02: I got bit by this also (see http://permalink.gmane.org/gmane.comp.programming.tools.cmake.user/17534 [^]). I also had the suggestion to inhibit expansion of quoted strings.

(I'll also note that, if 'foo' is supposed to contain the name of another variable, IMO the correct way to express this is 'if("${${foo}}" MATCHES "foo")'. I don't imagine it's common to want to expand something if it is the name of a variable or use it verbatim otherwise; even if so, we could use the unquoted case for that behavior.)

I'd also like to add that there is currently (2.6-2) an inconsistency; MATCHES expands, STREQUAL does not, although both are documented as expanding. This should also be fixed.
(0027875)
Andreas Schuh (reporter)
2011-11-30 17:20

I would like to note that I indeed ran into the problem that the construct

if ("${VAR}" STREQUAL "STR")

caused a bug in my CMake scripts because by incident the value of VAR was the name of another variable. Further, STR was a name of a variable as well, both defined outside the function where the if() statement occurred, and these two variables (${VAR} and STR) had the same value such that the condition evaluated to true. This was entirely unexpected.

Just shortly before, I replaced

if (VAR STREQUAL "STR")

by

if ("${VAR}" STREQUAl "STR")

to make sure that both sides (also the right hand side) would be interpreted as strings rather than names of variables. As it now turned out, this didn't help to prevent STREQUAL from misbehaving.

It should be possible to tell the STREQUAL operator clearly whether it should consider one or the other side as variable or string. Easiest would be to consider strings without quotes as possibly being names of variables, but to force using the string directly in the comparison if double quotes are given.

if (VAR STREQUAL "STR")
if ("STR" STREQUAL VAR)
if ("STR" STREQUAL "STR")
if (VAR STREQUAL VAR)

here VAR is always meant to be a CMake variable and hence not surrounded by double quotes and STR is an actual string value to compare to.

(using CMake 2.8.5)
(0027899)
Aaron C. Meadows (reporter)
2011-12-02 15:01

Ug. I totally just lost 3 days of development to this exact issue..

Having parsed a list of files into 3 lists, with slightly different handling for each, it took way too long to figure out why if("${_libListPtr}" STREQUAL "_dbg_libs") was almost always true for values _dbg_libs or _rel_libs but never for _gen_libs. Turns out it was because in my case, either _gen_libs has a list and _dbg_libs and _rel_libs are blank, or _gen_libs is blank and the other two have lists.

Really REALLY hard to track down... and neither --trace nor --debug-output seems to give any way to see this behavior.
(0034224)
jzarl (reporter)
2013-10-23 07:43
edited on: 2013-10-23 07:46

There seems to be some discussion on the cmake-devel list to fix auto-dereferencing:

http://public.kitware.com/pipermail/cmake-developers/2013-October/008717.html [^]

For me, the preferable solution would be to disable auto-dereferencing when quotes are used, but to force it if no quotes are used. Otherwise, people could still get bitten by it if they do something like the following:

if ( VARNAME STREQUAL "VARNAME" )

If auto-dereferencing is not forced for the unquoted LHS of the string comparison, this statement is ambiguous and is a highly likely source for introducing bugs later on. Just think of the situation when someone decides to unset VARNAME instead of setting it to e.g. FALSE.

(0036813)
Nils Gladitz (developer)
2014-09-16 07:16

Implemented by http://www.cmake.org/gitweb?p=cmake.git;a=commitdiff;h=188a1f23 [^]
(0038123)
Robert Maynard (manager)
2015-03-02 08:57

Closing resolved issues that have not been updated in more than 4 months.

- Issue History
Date Modified Username Field Change
2008-12-04 17:50 Bill Hoffman New Issue
2008-12-04 17:51 Bill Hoffman Status new => assigned
2008-12-04 17:51 Bill Hoffman Assigned To => Bill Hoffman
2008-12-04 18:36 mwoehlke Note Added: 0014293
2010-11-30 16:26 James Bigler Relationship added related to 0008823
2011-11-30 17:20 Andreas Schuh Note Added: 0027875
2011-12-02 15:01 Aaron C. Meadows Note Added: 0027899
2013-10-23 07:43 jzarl Note Added: 0034224
2013-10-23 07:46 jzarl Note Edited: 0034224 View Revisions
2014-09-16 07:16 Nils Gladitz Note Added: 0036813
2014-09-16 07:16 Nils Gladitz Status assigned => resolved
2014-09-16 07:16 Nils Gladitz Fixed in Version => CMake 3.1
2014-09-16 07:16 Nils Gladitz Resolution open => fixed
2015-03-02 08:57 Robert Maynard Note Added: 0038123
2015-03-02 08:57 Robert Maynard Status resolved => closed


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker