[PATCH] C++1y: Implement n3599

[Richard Smith]

I believe you'll need to update LookupLiteralOperator to handle this new
form appropriately. In particular, it looks like you'll currently reject
cases like:

  template<typename T, T...> int &operator""_x(); // #1
  float &operator""_x(const char*); // #2
  int &a = "foo"_x; // ok, calls #1
  float &b = 123_x; // ok, calls #2

It's probably best to repurpose the AllowRawAndTemplate flag into an enum
which specifies what form of literal operator you look for.


+      }else if (Params->size() == 2) {

Space before 'else'

+        // The second template parameter must be a parameter pack with the
+        // first template parameter as its type.
+        if (PmType && PmArgs &&
+            !PmType->isTemplateParameterPack() &&
+            PmArgs->isTemplateParameterPack()) {
+          const TemplateTypeParmType *TArgs =
+            dyn_cast<const
TemplateTypeParmType>(PmArgs->getType().getTypePtr());

You're dropping type qualifiers here. <typename T, const T...> is not an
acceptabe template-parameter-list.

+          if(TArgs && TArgs->getDecl() == PmType){

Space after 'if'.

+            Valid = true;
+            if (!getLangOpts().CPlusPlus1y)
+              Diag(FnDecl->getLocation(),
diag::warn_string_literal_operator_template);

This should be an ext_... not a warn_... (and should be a GNU warning not a
C++1y warning).

+          }
+        }


@@ -1538,12 +1536,53 @@ Sema::ActOnStringLiteral(const Token *StringToks,
unsigned NumStringToks,
+  DeclarationName OpName =
+    Context.DeclarationNames.getCXXLiteralOperatorName(UDSuffix);
+  DeclarationNameInfo OpNameInfo(OpName, UDSuffixLoc);
+  OpNameInfo.setCXXLiteralOperatorNameLoc(UDSuffixLoc);
+
+  QualType ArgTy[2];
+  ArgTy[0]=Context.getArrayDecayedType(StrTy);
+  ArgTy[1]=SizeType;

Spaces around '='. But please write this using braced initialization.

+
+  LookupResult R(*this, OpName, UDSuffixLoc, LookupOrdinaryName);
+  switch (LookupLiteralOperator(UDLScope, R, llvm::makeArrayRef(ArgTy, 2),
+                                           /*AllowRawAndTemplate*/true)) {
+
+  case LOLR_Cooked:
+  case LOLR_Raw: {

llvm_unreachable on the Raw case; that can't happen for a string.

+    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());
+  }
+
+  case LOLR_Template: {
+    TemplateArgumentListInfo ExplicitArgs;
+
+    unsigned CharBits = Context.getIntWidth(CharTy);
+    bool CharIsUnsigned = CharTy->isUnsignedIntegerType();
+    llvm::APSInt Value(CharBits, CharIsUnsigned);
+
+    TemplateArgument TypeArg(CharTy);

GCC uses the const-qualified element type here (it's not what I intended
when I wrote the paper, but it matches the wording, and it's what we need
to do for GCC compatibility). Either use CharTyConst here or persuade the
GCC folks to change their implementation =)

+    TemplateArgumentLocInfo
TypeArgInfo(Context.getTrivialTypeSourceInfo(CharTy));
+    ExplicitArgs.addArgument(TemplateArgumentLoc(TypeArg, TypeArgInfo));
+
+    for (unsigned I = 0, N = Lit->getLength(); I != N; ++I) {
+      Value = Lit->getCodeUnit(I);
+      TemplateArgument Arg(Context, Value, CharTy);
+      TemplateArgumentLocInfo ArgInfo(Lit);

Using Lit here isn't correct. We should be able to evaluate the expression
and get back the same integer value. If we actually need to specify this
for some reason, it would be better to synthesize a CharacterLiteral (and
point its source location at the relevant character from the source string
literal).

+      ExplicitArgs.addArgument(TemplateArgumentLoc(Arg, ArgInfo));
+    }
+    return BuildLiteralOperatorCall(R, OpNameInfo, None,
StringTokLocs.back(),

StringTokLocs.back() is not really right here. We could have any number of
string tokens forming this literal; ideally, we should store all their
locations on the UserDefinedLiteral expression.

+        &ExplicitArgs);
+  }
+  default:

Please mention LOLR_Error here, rather than using a default: clause. We
want a compile error if someone adds another LOLR kind and forgets to
handle it here.

+    return ExprError();
+  }


@@ -2992,11 +3031,11 @@ ExprResult Sema::ActOnNumericConstant(const Token
&Tok, Scope *UDLScope) {
       for (unsigned I = 0, N = Literal.getUDSuffixOffset(); I != N; ++I) {
         Value = TokSpelling[I];
         TemplateArgument Arg(Context, Value, Context.CharTy);
-        TemplateArgumentLocInfo ArgInfo;
+        TemplateArgumentLocInfo ArgInfo(Lit);

Again, this isn't right. Is this change observable somehow? This change
seems independent of N3599.

-      return BuildLiteralOperatorCall(R, OpNameInfo, None,
Tok.getLocation(),
-                                      &ExplicitArgs);
+      return BuildLiteralOperatorCall(R, OpNameInfo, None, TokLoc,
+          &ExplicitArgs);

The old indentation here is preferred (we line up to the '(' when breaking
a line in function arguments).


This needs more tests: in particular, please test that we pass the right
types and values as the template arguments, including for the cases where
we have an encoding-prefix or a raw string literal.


On Thu, Oct 3, 2013 at 12:32 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> Thanks for the patch.
>
> This proposal was not voted into C++1y, so it should not be controlled by
> that language flag, and the compatibility warning is incorrect. Instead,
> since GCC trunk has decided to ship this feature, it would make some sense
> to treat it as a GNU extension for now. (The response of EWG was more "not
> yet" than "no", so there's a good chance we'll have this feature in C++17.)
>
>
> On Wed, Oct 2, 2013 at 2:13 PM, Hristo Venev <mustrumr97 at gmail.com> wrote:
>
>> Literal operator templates for strings. Tests included.
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131003/7e76c2a5/attachment.html>