MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0015380CMakeCMakepublic2015-01-28 10:272015-07-08 08:57
ReporterAlina 
Assigned ToDaniele E. Domenichelli 
PrioritynormalSeverityminorReproducibilitysometimes
StatusclosedResolutionfixed 
PlatformLinuxOSUbuntuOS Version14.04
Product VersionCMake 2.8.12.2 
Target VersionCMake 3.3Fixed in VersionCMake 3.3 
Summary0015380: Misleading documentation in "Macro Argument Caveats" caveats
DescriptionThe issue was reproduced also on CMake 3.0.2 on OS X Yosemite.

Documentation points to the following :
"In the first case you can use if(${ARGV1}), in the second case, you can use foreach(loop_var ${ARGN}) but this will skip empty arguments. If you need to include them, you can use:"
http://www.cmake.org/cmake/help/v3.0/command/macro.html [^]

But if on any ARGVXXX is always false.

An simple example is attached.
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
related to 0013187closed Kitware Robot ARGVx is not reset in recursive function calls 
Attached Filestxt CMakeLists.txt (409) 2015-01-28 10:27
https://public.kitware.com/Bug/file/5369/CMakeLists.txt
Issue History
Date ModifiedUsernameFieldChange
2015-01-28 10:27AlinaNew Issue
2015-01-28 10:27AlinaFile Added: CMakeLists.txt
2015-01-28 10:36Brad KingNote Added: 0037852
2015-01-28 10:36Brad KingStatusnew => resolved
2015-01-28 10:36Brad KingResolutionopen => no change required
2015-01-28 10:39Brad KingNote Added: 0037853
2015-01-28 10:39Brad KingAssigned To => Daniele E. Domenichelli
2015-01-28 10:39Brad KingStatusresolved => new
2015-01-28 10:39Brad KingResolutionno change required => open
2015-01-28 10:39Brad KingSummaryOptional arguments for macros are ignored by if() => Misleading documentation in "Macro Argument Caveats" caveats
2015-01-28 10:39Brad KingStatusnew => assigned
2015-01-29 09:50Daniele E. DomenichelliNote Added: 0037858
2015-01-29 10:08Brad KingNote Added: 0037859
2015-02-06 11:03Daniele E. DomenichelliNote Added: 0037932
2015-02-06 11:21Brad KingNote Added: 0037933
2015-02-26 12:18Daniele E. DomenichelliNote Added: 0038079
2015-02-26 15:18Brad KingNote Added: 0038085
2015-02-26 16:34Stephen KellyRelationship addedrelated to 0013187
2015-02-27 09:00Daniele E. DomenichelliNote Added: 0038093
2015-02-27 11:18Brad KingNote Added: 0038097
2015-02-27 11:18Brad KingStatusassigned => resolved
2015-02-27 11:18Brad KingResolutionopen => fixed
2015-02-27 11:18Brad KingFixed in Version => CMake 3.3
2015-02-27 11:18Brad KingTarget Version => CMake 3.3
2015-07-08 08:57Robert MaynardNote Added: 0039085
2015-07-08 08:57Robert MaynardStatusresolved => closed

Notes
(0037852)
Brad King   
2015-01-28 10:36   
After macro argument replacement the if() sees

  if(pipo5)

which is false because no such variable is defined to a non-false constant. One can see this by adding

  set(pipo5 1)

before the macro invocation.
(0037853)
Brad King   
2015-01-28 10:39   
The documentation in the "Macro Argument Caveats" section was added here:

 Help: Document macro argument caveats in more detail
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f0db2e38 [^]

and needs to be clarified on this.
(0037858)
Daniele E. Domenichelli   
2015-01-29 09:50   
This is generated from a wrong expectation that

  if(${ARGV2})

can be used to tell if an argument was passed or not, but this will give you a wrong result even for a function, not just for a macro. What you want to use here is:

  if(NOT "${ARGV2}" STREQUAL "")


The only difference between functions and macro is that ARGV2 (not expanded) is not defined in a macro, but in a function it will be expanded like a normal variable. This means that there is difference ONLY when you use it without explicitly expanding it.


Just to clarify, the following tests:

  1. if(ARGV2)
  2. if(${ARGV2})
  3. if(NOT ARGV2 STREQUAL "")
  4. if(NOT "${ARGV2}" STREQUAL "")
  5. if(DEFINED ARGV2)
  6. if(DEFINED ${ARGV2})

With the following cases:

  a. Not passing an argument
  b. Passing an undefined argument
  c. Passing an argument that evaluates to TRUE (i.e. set(pipo5 1))
  d. Passing an argument that evaluates to FALSE (i.e. set(pipo5 0))

For

  M. MACRO
  F. FUNCTION



  1aM = 1aF: FALSE (ARGV2 not defined)
  2aM = 2aF: FALSE (${ARGV2} evaluates to "" that evaluates to FALSE)
  3aM = 3AF: TRUE (ARGV2 is not defined and therefore considered as the string "ARGV2" therefore the "NOT STREQUAL" evaluates to TRUE)
  4aM = 4AF: FALSE ("${ARGV2}" evaluates to "" therefore the "NOT STREQUAL" evaluates to FALSE)
  5aM = 5aF: FALSE (ARGV2 is not defined)
  6aM = 6aF: FALSE (${ARGV2} evaluates to "" that is not defined)

* 1bM: FALSE (ARGV2 not defined)
* 1bF: TRUE (ARGV2 evaluates to the string "pipo5" that is considered TRUE)
  2bM = 2bF: FALSE (pipo5 variable not defined)
* 3bM: TRUE (ARGV2 is not defined and therefore considered as the string "ARGV2" therefore the "NOT STREQUAL" evaluates to TRUE)
* 3bF: TRUE (ARGV2 evaluates to "pipo5" therefore the "NOT STREQUAL" evaluates to TRUE)
  4bM = 4bF: TRUE ("${ARGV2}" evaluates to "pipo5" therefore the "NOT STREQUAL" evaluates to TRUE)
* 5bM: FALSE (ARGV2 not defined)
* 5bF: TRUE (ARGV2 defined as "pipo5")
  6bM = 6bF: FALSE (${ARGV2} evaluates to "pipo5" that is not defined)

* 1cM: FALSE (ARGV2 not defined)
* 1cF: TRUE (ARGV2 evaluates to the string "pipo5" that is considered TRUE)
  2cM = 2cF: TRUE (${ARGV2} evaluates to 1 that is considered TRUE)
* 3cM: TRUE (ARGV2 is not defined and therefore considered as the string "ARGV2" therefore the "NOT STREQUAL" evaluates to TRUE)
* 3cF: TRUE (ARGV2 evaluates to "1" therefore the "NOT STREQUAL" evaluates to TRUE)
  4cM = 4cF: TRUE ("${ARGV2}" evaluates to "1" therefore the "NOT STREQUAL" evaluates to TRUE)
* 5cM: FALSE (ARGV2 not defined)
* 5cF: TRUE (ARGV2 defined as "pipo5")
  6cM = 6cF: TRUE (${ARGV2} evaluates to "pipo5" that is defined as "1")

* 1dM: FALSE (ARGV2 not defined)
* 1dF: TRUE (ARGV2 evaluates to the string "pipo5" that is considered TRUE)
  2dM = 2dF: FALSE (${ARGV2} evaluates to 0 that is considered FALSE)
* 3dM: TRUE (ARGV2 is not defined and therefore considered as the string "ARGV2" therefore the "NOT STREQUAL" evaluates to TRUE)
* 3cF: TRUE (ARGV2 evaluates to "0" therefore the "NOT STREQUAL" evaluates to TRUE)
  4cM = 4cF: TRUE ("${ARGV2}" evaluates to "0" therefore the "NOT STREQUAL" evaluates to TRUE)
* 5cM: FALSE (ARGV2 not defined)
* 5cF: TRUE (ARGV2 defined as "pipo5")
  6cM = 6cF: TRUE (${ARGV2} evaluates to "pipo5" that is defined as "0")



The only different behaviours here are 1, 3 (even though for a coincidence the result is always the same), and 5, i.e. the ones where ARGV2 is not explicitly expanded.

Note that 2b (the original issue) evaluates to FALSE both in a macro and in a function.
Also note that "set(pipo5 xxx)" will change the result of 2 (if set to something that evaluates to TRUE) and 6 (always) BOTH for macro and function.

Concluding, I don't think the "Macro Argument Caveats" section is this is misleading... Perhaps a section about how to check if an argument was set should be added both to the macro and function documentation instead?
(0037859)
Brad King   
2015-01-29 10:08   
Re 0015380:0037858:

> Perhaps a section about how to check if an argument was set

Yes, that makes sense. Thanks.
(0037932)
Daniele E. Domenichelli   
2015-02-06 11:03   
Actually I did some more tests and realized that

  if(NOT "${ARGV2}" STREQUAL "")

will not catch an empty ARGV2 (i.e. something like `foo(bar "")`).
Moreover in functions

  if(DEFINED ARGV2)

might be true even if the argument was not passed when calling a function from another function.

Therefore I believe that the proper way to check if an argument was set is to check the value of the variable ARGC, i.e.

  if(${ARGC} GREATER 2)

I updated the documentation based on my conclusion in the topic "macro-function-docs"

  Docs: Clarify how to check if an optional arg was passed to macro and function
  http://www.cmake.org/gitweb?p=stage/cmake.git;a=commitdiff;h=c76592a [^]

@Brad can you please review the change?
(0037933)
Brad King   
2015-02-06 11:21   
Re 0015380:0037932: Thanks. That proposes the wording:

----------------------------------------------------------
A common mistake is to perform tests on the ``ARGV#`` variable in order
to check if the ``ARGV#`` variable was passed as an extra argument, but
this might cause unexpected results.
The recommended way to perform such a check is to test the value of
``${ARGC}``:

<example code with lots of wrong cases>
----------------------------------------------------------

This tutorial-like wording does not belong in the reference documentation. Instead the wording describing the ARG* names should be updated to state explicitly that ARGV# beyond ARGC have undefined behavior.

While at this, please revise the main documentation paragraph text of macro() and function() to take advantage of reStructuredText markup a bit better.
(0038079)
Daniele E. Domenichelli   
2015-02-26 12:18   
0015380:0037933: Updated the branch, these are the new commits:

  Help: Refine the .rst formatting of macro and function documentation
  http://www.cmake.org/gitweb?p=stage/cmake.git;a=commit;h=e3363bf [^]

  Help: Clarify that ARGV# beyond ARGC will have an undefined behavior (0015380)
  http://www.cmake.org/gitweb?p=stage/cmake.git;a=commit;h=4efef3f [^]


I also had a look at CMake Modules fixed a few of them that were not checking ARGC and that could have caused an unexpected behaviour if called from inside another function:


  Check for ARGC before using ARGV#
  http://www.cmake.org/gitweb?p=stage/cmake.git;a=commit;h=5e76e22 [^]
(0038085)
Brad King   
2015-02-26 15:18   
Re 0015380:0038079: Thanks. I merged the two Help updates to 'next'.

The Modules updates should use

 if(ARGC GREATER ...)

in functions. The

 if(${ARGC} GREATER ...)

idiom is only for macros.
(0038093)
Daniele E. Domenichelli   
2015-02-27 09:00   
Re 0015380:0038085: Branch updated

  fixup! Check for ARGC before using ARGV#
  http://www.cmake.org/gitweb?p=stage/cmake.git;a=commitdiff;h=f9006aa [^]

Is there a difference for functions in using "${ARGC}" instead of "ARGC"? I usually choose the first form both in macro and functions, so that I don't have to worry if I switch from macro to function or vice versa.
(0038097)
Brad King   
2015-02-27 11:18   
Re 0015380:0038093: Thanks. I've squashed your fix in, fixed one typo, and merged the change to 'next' for testing:

 Modules: Check for ARGC before using ARGV#
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=a7631fc4 [^]

The if() command auto-dereferences variable names. See CMP0054. In a function(), ARGC is an actual variable we should name it to use the if() auto-dereference. In a macro() we have no choice but to use ${ARGC} because it is not really a variable.
(0039085)
Robert Maynard   
2015-07-08 08:57   
Closing resolved issues that have not been updated in more than 4 months.