[cfe-commits] [PATCH] for bug7505 with test cases, for initial review

Douglas Gregor dgregor at apple.com
Mon Jan 17 10:37:53 PST 2011


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



More information about the cfe-commits mailing list