[cfe-commits] [PATCH] for bug7505 (&foo<int>) post Doug's Feedback

[Douglas Gregor]

On Feb 19, 2011, at 12:36 PM, Faisal Vali wrote:

> 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.

I did a bit more stylistic tweaking, fixed one of the new tests to work on 64-bit platforms, and committed the result as r126048. Thanks! Please go ahead and close PR7505.

	- Doug

> 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
> <address-of-single-explicit-template.patch>