[cmake-developers] New EVIS parser moving forward (3.1)

[Jean-Christophe Fillion-Robin]

On Fri, Feb 21, 2014 at 12:05 PM, Ben Boeckel <ben.boeckel at kitware.com>wrote:

> On Fri, Feb 21, 2014 at 08:38:35 +0100, Rolf Eike Beer wrote:
> > > Are there any other corner cases I should consider banning as well?
> What
> > > about behavior that should be allowed that currently isn't?
> >
> > If this patch is the right place to address this is not clear to me, but
> I
> > mention them anyway.
>
> These are both if()-related behaviors. The EVIS parser *only* expands
> ${} and @@ variables. The treat-literal-as-variable name is implemented
> by cmIfCommand::GetVariableOrString by which point quoted-ness has been
> lost. The hex support would be in cmSystemTools::VersionCompare; the
> parser doesn't care at all if you use "literal" hex values. Hint: There
> are no integers in CMake; they're all strings and interpreted with atoi
> when necessary.
>
> > -if a string is quoted, it may still be expanded as a variable name as I
> have
> > heard, i.e. those 2 may act as the same:
> >
> > if ("a" STREQUAL b)
> > if (${a} STREQUAL ${b})
>
> Worse, the former may also really be either of:
>
> > if (a STREQUAL ${b})
> > if (${a} STREQUAL b)
>
> depending on which is defined.
>
> > I would not expect such behavior, in fact I consider this
> counterintuitive.
>
> I agree in general, however, my CMake style is to quote all variable
> expansions unless I want argument splitting. This rule would break code
> like this:
>
>     if ("CMAKE_${LANG}_FLAGS")
>     endif ()
>
> which would be unfortunate, IMO.


As a quick check, I ran the following search on CMake code base and it
returns nothing:

   cd ./CMake-src
   ack --cmake "\(\s*\"[a-zA-Z0-9]+\$"

Also nothing for Slicer, ITKv4, VTK6, ...

If possible, not having implicit expansion for quoted argument would be
great at make things more intuitive and practical.



> I don't think "quoted isn't expanded
> unless it is a single word with a nested variable expansion" is an
> obvious rule either. We could certainly make it so that anything with
> spaces isn't expanded which should take care of some of the more
> egregious cases. Maybe we should just deprecate implicit expansion in
> if()? That wouldn't be that hard of a policy to implement. I don't know
> how happy people would be with it though.
>




>
> Looking at this code, this expansion is missing --warn-uninitialized
> support. I don't know if it is worth it to implement it here since I
> imagine the false-positives would be legion.
>
> > -support for hex numbers would be cool, especially for parsing the
> version
> > numbers out of header files (like OpenSSL).
>
> This would be beneficial, I agree.
>
> --Ben
> --
>
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Please keep messages on-topic and check the CMake FAQ at:
> http://www.cmake.org/Wiki/CMake_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
>



-- 
+1 919 869 8849
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20140221/9962f0c9/attachment-0002.html>