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

[t.p.northover]

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.

[rsmith]

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.

[t.p.northover]

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]

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.

[t.p.northover]

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

[mclow.lists]

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

[mclow.lists]

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

clang/include/clang/Basic/DiagnosticLexKinds.td

def err_digit_separator_not_between_digits : Error<
   "digit separator cannot appear at %select{start|end}0 of digit sequence">;
 def warn_extraneous_char_constant : Warning<
   "extraneous characters in character constant ignored">;
 def warn_char_constant_too_large : Warning<
   "character constant too long for its type">;
 def err_multichar_utf_character_literal : Error<
   "Unicode character literals may not contain multiple characters">;
 def err_exponent_has_no_digits : Error<"exponent has no digits">;
-def ext_imaginary_constant : Extension<
-  "imaginary constants are a GNU extension">, InGroup<GNUImaginaryConstant>;
 def err_hex_constant_requires : Error<
   "hexadecimal floating %select{constant|literal}0 requires "
   "%select{an exponent|a significand}1">;
 def ext_hex_constant_invalid : Extension<
   "hexadecimal floating constants are a C99 feature">, InGroup<C99>;
 def ext_hex_literal_invalid : Extension<
   "hexadecimal floating literals are a C++1z feature">, InGroup<CXX1z>;
 def warn_cxx1z_hex_literal : Warning<

clang/include/clang/Basic/DiagnosticSemaKinds.td

 
 // Declarations.
 def ext_duplicate_declspec : ExtWarn<"duplicate '%0' declaration specifier">,
   InGroup<DuplicateDeclSpecifier>;
 def warn_duplicate_declspec : Warning<"duplicate '%0' declaration specifier">,
   InGroup<DuplicateDeclSpecifier>;
 def ext_plain_complex : ExtWarn<
   "plain '_Complex' requires a type specifier; assuming '_Complex double'">;
+def ext_imaginary_constant : Extension<
+  "imaginary constants are a GNU extension">, InGroup<GNUImaginaryConstant>;
 def ext_integer_complex : Extension<
   "complex integer types are a GNU extension">, InGroup<GNUComplexInteger>;
 
 def err_invalid_sign_spec : Error<"'%0' cannot be signed or unsigned">;
 def err_invalid_width_spec : Error<
   "'%select{|short|long|long long}0 %1' is invalid">;
 def err_invalid_complex_spec : Error<"'_Complex %0' is invalid">;
 def err_friend_decl_spec : Error<"'%0' is invalid in friend declarations">;

clang/include/clang/Sema/Sema.h

CXXMethodDecl *LookupMovingAssignment(CXXRecordDecl *Class, unsigned Quals,
                                         bool RValueThis, unsigned ThisQuals);
   CXXDestructorDecl *LookupDestructor(CXXRecordDecl *Class);
 
   bool checkLiteralOperatorId(const CXXScopeSpec &SS, const UnqualifiedId &Id);
   LiteralOperatorLookupResult LookupLiteralOperator(Scope *S, LookupResult &R,
                                                     ArrayRef<QualType> ArgTys,
                                                     bool AllowRaw,
                                                     bool AllowTemplate,
-                                                    bool AllowStringTemplate);
+                                                    bool AllowStringTemplate,
+                                                    bool DiagnoseMissing);
   bool isKnownName(StringRef name);
 
   void ArgumentDependentLookup(DeclarationName Name, SourceLocation Loc,
                                ArrayRef<Expr *> Args, ADLResult &Functions);
 
   void LookupVisibleDecls(Scope *S, LookupNameKind Kind,
                           VisibleDeclConsumer &Consumer,
                           bool IncludeGlobalScope = true);

clang/lib/Lex/LiteralSupport.cpp

case 'I':
           default:
             break;
           }
         }
         if (MicrosoftInteger) {
           assert(s <= ThisTokEnd && "didn't maximally munch?");
           break;
         }
       }
-      // "i", "if", and "il" are user-defined suffixes in C++1y.
-      if (*s == 'i' && PP.getLangOpts().CPlusPlus14)
-        break;
       // fall through.
     case 'j':
     case 'J':
       if (isImaginary) break;   // Cannot be repeated.
       isImaginary = true;
       ImaginarySuffixLoc = s;
       continue;  // Success.
     }
     // If we reached here, there was an error or a ud-suffix.
     break;
   }
 
-  if (s != ThisTokEnd) {
+  if (s != ThisTokEnd || isImaginary) {
     // FIXME: Don't bother expanding UCNs if !tok.hasUCN().
     expandUCNs(UDSuffixBuf, StringRef(SuffixBegin, ThisTokEnd - SuffixBegin));
     if (isValidUDSuffix(PP.getLangOpts(), UDSuffixBuf)) {
       // Any suffix pieces we might have parsed are actually part of the
       // ud-suffix.
-      isLong = false;
-      isUnsigned = false;
-      isLongLong = false;
-      isFloat = false;
-      isHalf = false;
-      isImaginary = false;
-      MicrosoftInteger = 0;
-
       saw_ud_suffix = true;
       return;
     }
 
+    if (s != ThisTokEnd) {
       // Report an error if there are any.
       PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, SuffixBegin - ThisTokBegin),
               diag::err_invalid_suffix_constant)
-      << StringRef(SuffixBegin, ThisTokEnd-SuffixBegin) << isFPConstant;
+          << StringRef(SuffixBegin, ThisTokEnd - SuffixBegin) << isFPConstant;
       hadError = true;
-    return;
     }
-
-  if (isImaginary) {
-    PP.Diag(PP.AdvanceToTokenCharacter(TokLoc,
-                                       ImaginarySuffixLoc - ThisTokBegin),
-            diag::ext_imaginary_constant);
+    return;
   }
 }
 
 /// ParseDecimalOrOctalCommon - This method is called for decimal or octal
 /// numbers. It issues an error for illegal digits, and handles floating point
 /// parsing. If it detects a floating point number, the radix is set to 10.
 void NumericLiteralParser::ParseDecimalOrOctalCommon(SourceLocation TokLoc){
   assert((radix == 8 || radix == 10) && "Unexpected radix");

clang/lib/Sema/SemaExpr.cpp

static ExprResult BuildCookedLiteralOperatorCall(Sema &S, Scope *Scope,
 
   DeclarationName OpName =
     S.Context.DeclarationNames.getCXXLiteralOperatorName(UDSuffix);
   DeclarationNameInfo OpNameInfo(OpName, UDSuffixLoc);
   OpNameInfo.setCXXLiteralOperatorNameLoc(UDSuffixLoc);
 
   LookupResult R(S, OpName, UDSuffixLoc, Sema::LookupOrdinaryName);
   if (S.LookupLiteralOperator(Scope, R, llvm::makeArrayRef(ArgTy, Args.size()),
-                              /*AllowRaw*/false, /*AllowTemplate*/false,
-                              /*AllowStringTemplate*/false) == Sema::LOLR_Error)
+                              /*AllowRaw*/ false, /*AllowTemplate*/ false,
+                              /*AllowStringTemplate*/ false,
+                              /*DiagnoseMissing*/ true) == Sema::LOLR_Error)
     return ExprError();
 
   return S.BuildLiteralOperatorCall(R, OpNameInfo, Args, LitEndLoc);
 }
 
 /// ActOnStringLiteral - The specified tokens were lexed as pasted string
 /// fragments (e.g. "foo" "bar" L"baz").  The result string has to handle string
 /// concatenation ([C99 5.1.1.2, translation phase #6]), so it may come from
Sema::ActOnStringLiteral(ArrayRef<Token> StringToks, Scope *UDLScope) {
   OpNameInfo.setCXXLiteralOperatorNameLoc(UDSuffixLoc);
 
   QualType ArgTy[] = {
     Context.getArrayDecayedType(StrTy), SizeType
   };
 
   LookupResult R(*this, OpName, UDSuffixLoc, LookupOrdinaryName);
   switch (LookupLiteralOperator(UDLScope, R, ArgTy,
-                                /*AllowRaw*/false, /*AllowTemplate*/false,
-                                /*AllowStringTemplate*/true)) {
+                                /*AllowRaw*/ false, /*AllowTemplate*/ false,
+                                /*AllowStringTemplate*/ true,
+                                /*DiagnoseMissing*/ true)) {
 
   case LOLR_Cooked: {
     llvm::APInt Len(Context.getIntWidth(SizeType), Literal.GetNumStringChars());
     IntegerLiteral *LenArg = IntegerLiteral::Create(Context, Len, SizeType,
                                                     StringTokLocs[0]);
     Expr *Args[] = { Lit, LenArg };
 
     return BuildLiteralOperatorCall(R, OpNameInfo, Args, StringTokLocs.back());
if (Literal.hasUDSuffix()) {
     OpNameInfo.setCXXLiteralOperatorNameLoc(UDSuffixLoc);
 
     SourceLocation TokLoc = Tok.getLocation();
 
     // Perform literal operator lookup to determine if we're building a raw
     // literal or a cooked one.
     LookupResult R(*this, OpName, UDSuffixLoc, LookupOrdinaryName);
     switch (LookupLiteralOperator(UDLScope, R, CookedTy,
-                                  /*AllowRaw*/true, /*AllowTemplate*/true,
-                                  /*AllowStringTemplate*/false)) {
-    case LOLR_Error:
+                                  /*AllowRaw*/ true, /*AllowTemplate*/ true,
+                                  /*AllowStringTemplate*/ false,
+                                  /*DiagnoseMissing*/ !Literal.isImaginary)) {
+    case LOLR_Error: {
+      // Lookup failure for imaginary constants isn't fatal, there's still the
+      // GNU extension producing _Complex types.
+      if (Literal.isImaginary)
+        break;
       return ExprError();
-
+    }
     case LOLR_Cooked: {
       Expr *Lit;
       if (Literal.isFloatingLiteral()) {
         Lit = BuildFloatingLiteral(*this, Literal, CookedTy, Tok.getLocation());
       } else {
         llvm::APInt ResultVal(Context.getTargetInfo().getLongLongWidth(), 0);
         if (Literal.GetIntegerValue(ResultVal))
           Diag(Tok.getLocation(), diag::err_integer_literal_too_large)
if (Literal.GetIntegerValue(ResultVal)) {
 
       if (ResultVal.getBitWidth() != Width)
         ResultVal = ResultVal.trunc(Width);
     }
     Res = IntegerLiteral::Create(Context, ResultVal, Ty, Tok.getLocation());
   }
 
   // If this is an imaginary literal, create the ImaginaryLiteral wrapper.
-  if (Literal.isImaginary)
+  if (Literal.isImaginary) {
     Res = new (Context) ImaginaryLiteral(Res,
                                         Context.getComplexType(Res->getType()));
 
+    Diag(Tok.getLocation(), diag::ext_imaginary_constant);
+  }
   return Res;
 }
 
 ExprResult Sema::ActOnParenExpr(SourceLocation L, SourceLocation R, Expr *E) {
   assert(E && "ActOnParenExpr() missing expr");
   return new (Context) ParenExpr(L, R, E);
 }
 

clang/lib/Sema/SemaLookup.cpp

 ///
 /// Normal overload resolution is not used to select which literal operator to
 /// call for a user-defined literal. Look up the provided literal operator name,
 /// and filter the results to the appropriate set for the given argument types.
 Sema::LiteralOperatorLookupResult
 Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
                             ArrayRef<QualType> ArgTys,
                             bool AllowRaw, bool AllowTemplate,
-                            bool AllowStringTemplate) {
+                            bool AllowStringTemplate, bool DiagnoseMissing) {
   LookupName(R, S);
   assert(R.getResultKind() != LookupResult::Ambiguous &&
          "literal operator lookup can't be ambiguous");
 
   // Filter the lookup results appropriately.
   LookupResult::Filter F = R.makeFilter();
 
   bool FoundRaw = false;
Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
 
   if (FoundTemplate)
     return LOLR_Template;
 
   if (FoundStringTemplate)
     return LOLR_StringTemplate;
 
   // Didn't find anything we could use.
+  if (DiagnoseMissing)
     Diag(R.getNameLoc(), diag::err_ovl_no_viable_literal_operator)
         << R.getLookupName() << (int)ArgTys.size() << ArgTys[0]
         << (ArgTys.size() == 2 ? ArgTys[1] : QualType()) << AllowRaw
         << (AllowTemplate || AllowStringTemplate);
   return LOLR_Error;
 }
 
 void ADLResult::insert(NamedDecl *New) {
   NamedDecl *&Old = Decls[cast<NamedDecl>(New->getCanonicalDecl())];
 
   // If we haven't yet seen a decl for this key, or the last decl
   // was exactly this one, we're done.

clang/test/SemaCXX/imaginary-constants.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s -include %s -std=gnu++98
+// RUN: %clang_cc1 -fsyntax-only -verify %s -include %s -std=c++11
+// RUN: %clang_cc1 -fsyntax-only -verify %s -include %s -std=c++14 -DCXX14=1
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+_Complex int val1 = 2i;
+_Complex long val2 = 2il;
+_Complex long long val3 = 2ill;
+_Complex float val4 = 2.0if;
+_Complex double val5 = 2.0i;
+_Complex long double val6 = 2.0il;
+
+#if CXX14
+
+#pragma clang system_header
+
+namespace std {
+  template<typename T> struct complex {};
+  complex<float> operator""if(unsigned long long);
+  complex<float> operator""if(long double);
+
+  complex<double> operator"" i(unsigned long long);
+  complex<double> operator"" i(long double);
+
+  complex<long double> operator"" il(unsigned long long);
+  complex<long double> operator"" il(long double);
+}
+
+using namespace std;
+
+complex<float> f1 = 2.0if;
+complex<float> f2 = 2if;
+complex<double> d1 = 2.0i;
+complex<double> d2 = 2i;
+complex<long double> l1 = 2.0il;
+complex<long double> l2 = 2il;
+
+#endif
+
+#endif