1730960 - Preprocessor should fail on, or handle, '# ifdef' (space between # and ifdef)

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P3)

Description

[Mark Banner (:standard8)]

We've just found two long standing bugs (2yr) in our preferences (see "See Also") which were both caused by code in StaticPrefsList.yaml of this form:

# ifdef XP_WIN
  value: 1
# else
  value: 0
# endif

There is even a warning at the start of this file:

# Note finally that this file is preprocessed by preprocessor.py, not the C++
# preprocessor. As a result, the following things may be surprising.
...
# - Spaces aren't permitted between the leading '#' and the name of a
#   directive, e.g. `#ifdef XYZ` works but `# ifdef XYZ` does not.

We should either make the preprocessor fail on ifdefs with a space, or it should handle them.

Comment 1

[[:sergesanspaille]]

Attachment: Bug 1730960 - Allow spaces between marker and preprocessor, and change comment marker command r=glandium

This way it matches the C pre processor behavior many people are used
to.

The way this change is implemented also allows for non-command comments
to exist, e.g.

    #hello

Used to be an error and it no longer is, it's just copy-pasted.

Comment 2

[[:sergesanspaille]]

Attachment: Bug 1730960 - Detect ambiguous preprocessor.py commands r=glandium

Because the preprocessor handles both comments in the form of

some comment

And commands in the form of

#some command

And due to C preprocessor heritage, people tend to write

some command

which is considered a comment and not a command. Detect this situation
and generate an hard error when meeting such ambiguous commands.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51302571bf80
Detect ambiguous preprocessor.py commands r=glandium

Comment 4

[Sandor Molnar[:smolnar]]

Backed out for causing build bustages in modules/libpref/init/StaticPrefList.yaml

Backout link: https://hg.mozilla.org/integration/autoland/rev/6fc5ecc39bd6a902e822e503c098eee4c2aeb08e

Push with failures

Failure log -> mozbuild.preprocessor.Error: ('$SRCDIR/modules/libpref/init/StaticPrefList.yaml', 13635, 'AMBIGUOUS_COMMENT', '# ifdef MOZ_WMF\n')

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ee47853c14c
Detect ambiguous preprocessor.py commands r=glandium

Comment 6

[Sandor Molnar[:smolnar]]

Backed out for causing build bustages in modules/libpref/init/StaticPrefList.yaml

Backout link: https://hg.mozilla.org/integration/autoland/rev/8289adf7c91fe94c278a9778346187961597dea2

Push with failures

Failure log -> mozbuild.preprocessor.Error: ('$SRCDIR/modules/libpref/init/StaticPrefList.yaml', 1753, 'AMBIGUOUS_COMMENT', '# if they were issued by Window.fetch (and similar).\n')

Comment 7

[[:sergesanspaille]]

Patch updated and pushed again. Sorry for the noise.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34f3d7c398a2
Detect ambiguous preprocessor.py commands r=glandium

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/34f3d7c398a2