[PATCH] C++1y: Implement n3599
Hristo Venev
mustrumr97 at gmail.com
Fri Oct 4 11:39:51 PDT 2013
I think I fixed most bugs from the first version of the patch.
I tried checking if the parameters' type hasQualifiers() on definition and
it's always false. Why?
I don't think the type argument should be const-qualified so I'll ask the
gcc folks what they think.
BuildLiteralOperatorCall is given LitEndTok so StringTokLocs.back() should
be right.
The TokLoc change is because Tok.getLocation() is always called. Now it's
only called at one place.
I also added some raw string/unicode tests and they seem to work as
expected.
On Fri, Oct 4, 2013 at 12:06 AM, Richard Smith <richard at metafoo.co.uk>wrote:
> 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/20131004/53d1b164/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-C-1y-Implement-n3599-v2.patch
Type: application/octet-stream
Size: 22075 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131004/53d1b164/attachment.obj>
More information about the cfe-commits
mailing list