search

bloomberg / bde

Basic Development Environment - a set of foundational C++ libraries used at Bloomberg.
Apache License 2.0
1.68k stars 318 forks source link

crazy #undef #255

Closed seanbaxter closed 4 years ago

seanbaxter commented 4 years ago

bde/groups/bsl/bsls/bsls_libraryfeatures.h:1152:77

undef BSLS_LIBRARYFEATURES_HAS_CPP11_PAIR_PIECEWISE_CONSTRUCTOR 1

Breaks my compiler. Is this supposed to be an #undef without the 1 or a #define? What you have now is illegal syntax.

notadragon commented 4 years ago

bde/groups/bsl/bsls/bsls_libraryfeatures.h:1152:77

undef BSLS_LIBRARYFEATURES_HAS_CPP11_PAIR_PIECEWISE_CONSTRUCTOR 1

Breaks my compiler. Is this supposed to be an #undef without the 1 or a #define? What you have now is illegal syntax.

I would be curious why you are attempting to parse this line.. it's in a block guarded for a beta Sun compiler (and specifically a version of that compiler which even we are not using yet). I'm not saying it would work on that compiler - just that only a compiler where BSLS_PLATFORM_CMP_SUN gets defined and BSLS_PLATFORM_CMP_VERSION >= 0x5150 is true should this be considered ill-formed.

osubboo commented 4 years ago

The fix is merged into the internal base. Will be synced within an hour.

seanbaxter commented 4 years ago

It's ill-formed no matter what the macros are.

I'm tokenizing those lines because my first pass breaks the text into a tree of nodes. Each node is either an uninterrupted block of text or a directive. #if/ifdef directive nodes have children. This lets me perform preprocessor control flow by navigating a tree or directives rather than having to read text. Macro include guards and the like are trivially captured by this structure.

The values of macros are used when traversing the tree to emit C++ tokens (as opposed to PP tokens). The BSLS_PLATFORM_CMP_SUN prevents the compiler from executing that directive, not from parsing it.

notadragon commented 4 years ago

Nope, that line is not a valid control-line (it does not match any of the productions in [cpp.pre]/1), but that just makes it a text-line part of the group under the #ifdef. The group is skipped, so nothing is ill-formed just because that line is not parsable as anything useful.

That bit of functionality is important in case future standards or conforming compiler extensions on other compilers add meaning to such a directive.

And [cpp.pre]/p3 more explicitly states what i'm saying: When in a group that is skipped (15.2), the directive syntax is relaxed to allow any sequence of preprocessing tokens to occur between the directive name and the following new-line character.

hyrosen commented 4 years ago

It's not a text line: A sequence of preprocessing tokens is only a text-line if it does not begin with a directive-introducing token.

However: When in a group that is skipped ([cpp.cond]), the directive syntax is relaxed to allow any sequence of preprocessing tokens to occur between the directive name and the following new-line character.

From: reply@reply.github.com At: 03/04/20 12:52:12To: bde@noreply.github.com Cc: subscribed@noreply.github.com Subject: Re: [bloomberg/bde] crazy #undef (#255)

Nope, that line is not a valid control-line (it does not match any of the productions in [cpp.pre]/1), but that just makes it a text-line part of the group under the #ifdef. The group is skipped, so nothing is ill-formed just because that line is not parsable as anything useful. That bit of functionality is important in case future standards or conforming compiler extensions on other compilers add meaning to such a directive.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

RMGiroux commented 4 years ago

Whether or not circle should have ignored them, we're still grateful you found them - those lines were just silly.

They're fixed now, and the fix has propagated to github. Thanks!