[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