Page MenuHomePhabricator

Warn when using `defined` in a macro definition.
ClosedPublic

Authored by thakis on Jan 4 2016, 12:38 PM.

Details

Reviewers
rsmith
Summary

As far as I can tell, doing

#define HAVE_FOO_BAR defined(FOO) && defined(BAR)
#if HAVE_FOO
...
#endif

has undefined behavior per [cpp.cond]p4. In practice, it can have different behavior in gcc and Visual Studio – see the comment in PPExpressions.cpp. So we should warn on this.

One problem is that this also applies to function-like macros. While the example above can be written like

#if defined(FOO) && defined(BAR)
#defined HAVE_FOO 1
#else
#define HAVE_FOO 0
#endif

there is no easy way to rewrite a function-like macro like #define FOO(x) (defined __foo_##x && __foo_##x). Function-like macros like this are used in practice, and compilers seem to not have differing behavior in that case. So make this a default-on warning only for object-like macros and an extension warning that only shows up with pedantic for function-like macros. (But it's undefined behavior in both cases.)

Diff Detail

Event Timeline

thakis updated this revision to Diff 43912.Jan 4 2016, 12:38 PM
thakis retitled this revision from to Warn when using `defined` in a macro definition..
thakis updated this object.
thakis added a reviewer: rsmith.
thakis added a subscriber: cfe-commits.
rsmith accepted this revision.Jan 14 2016, 11:40 AM
rsmith edited edge metadata.
rsmith added inline comments.
lib/Lex/PPExpressions.cpp
104–105

Move this down to the end of the function, after we've checked that we have a syntactically valid defined operator, to avoid duplicate diagnostics on a case like:

#define FOO defined(
#if FOO
This revision is now accepted and ready to land.Jan 14 2016, 11:40 AM
thakis updated this revision to Diff 44958.Jan 14 2016, 7:33 PM
thakis edited edge metadata.

For function-type macros, make the warning only show up with -pedantic

thakis updated this object.Jan 14 2016, 7:34 PM

Thanks for the review!

I tweaked it a bit so that this only warns on function-type macros in
-pedantic mode. That way, the warning can even be used in practice :-)

thakis updated this revision to Diff 44959.Jan 14 2016, 7:39 PM

Address review comment.

thakis marked an inline comment as done.Jan 14 2016, 7:40 PM

Also addressed your comment and added a test for that.