569373 - make Preprocessor.py handle -DVAR=NUMBER such that #if VAR == NUMBER works

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

Attachment: support -DFOO=NUMBER better

Currently if you do:
DEFINES += -DFOO=100

and then in your preprocessed file:
#if FOO=100
some text
#endif

The #if condition will never be true. This is because the expression parser (Expression.py) calls int() on the values it parses out, but the commandline handler for Preprocessor.py puts string values in for variables from the commandline. (preprocessor.pl appears to handle all variable values as strings, FWIW).

This simple patch makes the commandline handler treat all-digit values as ints, and includes a couple of tests.

Comment 1

[Axel Hecht [:Pike]]

Comment on attachment 448546 [details] [diff] [review]
support -DFOO=NUMBER better

This is assuming that in the JarMaker.py case, we're forcing our defines to be strings, and that's what it is, right? (unescapeDefines is only true for JarMaker.py)

r- based on the detail that 0100 should be parsed as octal, as that's what #define does, see http://hg.mozilla.org/mozilla-central/file/4e549a25e56f/config/Preprocessor.py#l244.

Which is, oh shiny, inconsistent to what Expressions, do. So

#define FOO 0100
#if FOO==0100
0100 not shown
#endif
#if FOO==64
64 shown
#endif

Happy faces, anyone? Should that be fixed?

We should make https://developer.mozilla.org/en/Build/Text_Preprocessor#Conditionals document the behaviour we end up with, too :-)

Comment 2

[(not currently active) Ted Mielczarek]

Attachment: (v2) support -DFOO=NUMBER better, also support octal

Lame, but I fixed it.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 448585 [details] [diff] [review]
(v2) support -DFOO=NUMBER better, also support octal

http://hg.mozilla.org/mozilla-central/rev/8385a247ab0c

Comment 4

[ISHIKAWA, Chiaki]

I forgot to mention that the problem manifested itself after the
latest change:
$ hg log nsHelperAppDlg.js
changeset:   43659:8f63228de64c
user:        Gavin Sharp <[e-mail]>
date:        Mon Jun 07 10:31:58 2010 -0400
summary:     Bug 570155: Make the helperApp dialog use the getFileDisplayname helper consistently (fixes a broken #ifdef exposed by bug 569373), and refactor some ifdefs to avoid code duplication, r=Neil

This fix probably brings a dormant bug to the light of the day.
(That is for some reason, this.dialog was not null for sometime?)