Page MenuHomePhabricator

Lexer: allow imaginary constants in GNU mode (only).
ClosedPublic

Authored by t.p.northover on May 22 2017, 3:29 PM.

Details

Reviewers
rsmith
Summary

While looking into the regression tests' compatibility with C++14, many of the failures were because that specification defined UDLs for imaginary constants, specifically ones ending in 'i' and 'il'. This conflicted with the previous GNU extension that allowed them purely as a builtin literal syntax so they'd been disabled in C++14 mode.

GCC's behaviour is to use the builtin constants in all "-std=gnu++N" modes but disable them for "-std=c++11" and "-std=c++14" (they are still permitted in "-std=c++98"). This seemed inconsistent (and likely an implementation detail rather than intended behaviour) so my patch makes the decision purely on whether we're in GNU mode.

Does it look reasonable?

Tim.

Diff Detail

Event Timeline

t.p.northover created this revision.May 22 2017, 3:29 PM
rsmith edited edge metadata.May 22 2017, 5:17 PM

Counterproposal: in -std=*++14 onwards, treat this construct as a user-defined literal, but fall back on the built-in interpretation if name lookup doesn't find an operator""i function. (The two interpretations only conflict if the source code explicitly does something like using namespace std::complex_literals;, which seems like a pretty big clue that the user expected 1.0ito be std::complex<double> rather than _Complex double.)

If we find that's not enough for GCC compatibility in practice (perhaps in old code using using namespace std;), we can consider whether we want to make -std=gnu++* cause the built-in interpretation of the i* suffixes to be preferred over the overloaded interpretations.

clang/lib/Lex/LiteralSupport.cpp
654–655

I'm unconvinced this is sufficient justification to remove this previously-supported extension from -std=c* and -std=c++98 modes.

OK, that seems doable. After playing around I came up with this new patch (and remembered to "git add" my own test this time).

It feels like a bit of a hack, but I played around with some alternatives and couldn't come up with anything better. What do you think?

rsmith accepted this revision.May 23 2017, 1:04 PM

I like it, and it seems like it would nicely generalize if there are more cases that we find need this treatment.

clang/lib/Lex/LiteralSupport.cpp
676–682

We should still do this if s != ThisTokEnd, rather than relying on isValidUDSuffix to also validate that we have a valid built-in suffix. (Eg, if C++20 adds an "in" suffix for "inches", this patch would treat "1.0in" as a valid _Complex double literal if no operator""in is found, because we don't validate that all the suffix characters are used on the fallback path.)

684

Redundant return. Remove? :)

clang/lib/Sema/SemaExpr.cpp
3355

I'd feel a bit more comfortable with a different return value for "lookup failed but we've not issued an error", to make it more obvious that we never return ExprError() here without issuing a diagnostic.

This revision is now accepted and ready to land.May 23 2017, 1:04 PM
t.p.northover closed this revision.May 23 2017, 2:43 PM

Thanks Richard, I've committed it as r303694 with your suggestions.

This broke a libc++ test. The following is expected to fail to compile:

#include <complex>
#include <cassert>

int main()
{
    std::complex<float> foo  = 1.0if;  // should fail w/conversion operator not found
}

when build as C++1z

More. Trying the above code on godbolt.org, gcc 6.1/6.2/6.3/7.1 all reject it (with -std=c++14 and -std=c++1z) with the error message:

<source>: In function 'int main()':
<source>:4:30: error: unable to find numeric literal operator 'operator""if'

{ std::complex<float> foo  = 1.0if; }
                             ^~~~~

<source>:4:30: note: use -std=gnu++11 or -fext-numeric-literals to enable more built-in suffixes
Compiler exited with result code 1