[cfe-commits] [PATCH] for bug7505 (&foo<int>) post Doug's Feedback
Faisal Vali
faisalv at gmail.com
Sat Feb 19 12:36:15 PST 2011
Hi Doug,
Here is the updated patch with stylistic changes addressed -
including having 'ResolveAndFixSingleFunctionTemplateSpecialization'
return an ExprResult instead of 'true' or 'false'.
Once again, I tested against CXX, SemaCXX, SemaTemplate, Sema.
Thanks!
Faisal Vali
On Mon, Jan 17, 2011 at 12:37 PM, Douglas Gregor <dgregor at apple.com> wrote:
> Hi Faisal,
>
> On Dec 12, 2010, at 9:39 PM, Faisal Vali wrote:
>
>> Once again Doug - sorry for the delay.
>>
>> To recap, this patch is intended to completely fix bug7505 and allow
>> the following code:
>> template<class T> void f();
>> template<class T, class U> T f(U);
>>
>> bool b = f<int>;
>> int i = (int) f<int>;
>>
>> I tried two approaches in fixing this bug - patching BuildTemplateID
>> vs patching each of the casts and the standard conversions.
>> Patching BuildTemplateId did not work because it would bypass overload
>> resolution (unless i detect trailing lparens) in certain cases and
>> create confusing results.
>
> I think this is the right approach. Technically, the patch looks good, but there are some stylistic things that need adjustment before it can go in.
>
> +static bool ResolveAndFixSingleFunctionTemplateSpecialization(Sema &Self
> + , Expr *&SrcExpr
> + , bool DoFunctionPointerConverion = false
> + , bool Complain = false
> + , const SourceRange& OpRangeForComplaining = SourceRange()
> + , QualType DestTypeForComplaining = QualType()
> + , unsigned DiagIDForComplaining = 0
> + );
> +
> +
>
> Commas at the end of the lines, please, and we tend to right-justify parameters. Same comment about various argument lists throughout the patch; please take a look at http://llvm.org/docs/CodingStandards.html .
>
> More importantly, Clang functions typically return "true" on failure and false on success, and this function goes against that convention. How about changing SrcExpr to just be an Expr*, and returning an ExprResult?
>
> - Doug
-------------- next part --------------
A non-text attachment was scrubbed...
Name: address-of-single-explicit-template.patch
Type: application/octet-stream
Size: 47805 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110219/d6414d5a/attachment.obj>
More information about the cfe-commits
mailing list