[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