Hi Faisal,
Were you planning to finish up this patch? It's definitely going in the right
direction.
- Doug
On Sep 20, 2010, at 11:41 AM, Douglas Gregor wrote:
>
> On Sep 10, 2010, at 3:26 PM, Faisal Vali wrote:
>
>> The patch fixes the following related bugs:
>>
>>
>> 1) Fix bug http://llvm.org/bugs/show_bug.cgi?id=7505
>> (overload resolution of &foo<int> when it identifies a single function)
>>
>> template<class T> void foo(T);
>> bool b = &foo<int>; // now, ok.
>>
>> -- added a call to Sema::ResolveSingleFunctionTemplateSpecialization in
>> Sema::ResolveAddressOfOverloadedFunction prior to any other deduction
>> or resolution to check to see if the name identifies a single function
>> (SemaOverload.cpp)
>
> This is:
>
> @@ -6225,19 +6244,49 @@
> OverloadExpr::FindResult Ovl = OverloadExpr::find(From);
> OverloadExpr *OvlExpr = Ovl.Expression;
>
> + // template<class T> foo(T); &foo<int>; can only identify one function
> + // C++0x [temp.arg.explicit]p3:
> + // [...] In contexts where deduction is done and fails, or in contexts
> + // where deduction is not done, if a template argument list is
> + // specified and it, along with any default template arguments,
> + // identifies a single function template specialization, then the
> + // template-id is an lvalue for the function template specialization.
> + if (FunctionType->isBooleanType() && OvlExpr->hasExplicitTemplateArgs())
> + {
> + if (FunctionDecl* Fun =
> ResolveSingleFunctionTemplateSpecialization(From,
> + Complain, &FoundResult) )
> + {
> + MarkDeclarationReferenced(From->getLocStart(), Fun);
> + if (Complain) {
> + CheckUnresolvedAccess(*this, OvlExpr, FoundResult);
> + DiagnoseUseOfDecl(FoundResult, OvlExpr->getNameLoc());
> + }
> + return Fun;
> + }
> + return 0;
> + }
> +
>
> but it looks like it's in the wrong place. [temp.arg.explicit]p3 only kicks
> in where deduction is not done or where it fails, so it seems that this code
> should come at the end of this routine, after we've tried (and failed) to do
> deduction.
>
> Also, I don't understand why we have the FunctionType->isBooleanType() check,
> which is rather... unintuitive. Why check for 'bool' and not 'int', for
> example?
>
>> -- modified Sema.h to add a 'Complain' flag to getMostSpecialized and
>> ResolveSingleFunctionTemplateSpecialization so that we can percolate
>> it through to these other functions that do Complain even if we
>> don't intend to from ResolveAddressOfOverloadedFunction
>> (required adding another call to ResolveOverload in SemaExpr.cpp with
>> Complain set to true, since PerformImplicitConversion was calling
>> it with 'false' yet the function was still complaining)
>> (Sema.h, SemaTemplateDeduction.cpp)
>
> Okay.
>
>> -- made some cosmetic changes (left the major refactoring for the next
>> revision)
>> to ResolveSingleFunctionTemplateSpecialization
>>
>>
>>
>> 2) Fix invalid conversion of an unresolved overloaded name into bool via '!'
>> template<class T> void foo(T);
>> bool b = !&foo; // this should be ambiguous
>> -- Added a call to PerformContextuallyConvertToBool in CreateBuiltinUnaryOp
>> to ensure that the OverloadTy does not leak through (semaExpr.cpp)
>
> That's is code:
>
> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp (revision 113599)
> +++ lib/Sema/SemaExpr.cpp (working copy)
> @@ -6771,9 +6781,26 @@
>
> resultType = Input->getType();
> if (resultType->isDependentType())
> break;
> - if (!resultType->isScalarType()) // C99 6.5.3.3p1
> +
> + if (!resultType->isScalarType())
> + {
> return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
> << resultType << Input->getSourceRange());
> + }
> + // ensure that it is convertible to bool
> + // i.e weed out '!&f' when &f is overloaded, but allow &f<int>
> + if (PerformContextuallyConvertToBool(Input)) // C99 6.5.3.3p1
> + {
> + if (resultType == Context.OverloadTy)
> + {
> + DeclAccessPair access;
> + // Call it again (initially was thru PerformContext) but now Complain
> + ResolveAddressOfOverloadedFunction(Input, Context.BoolTy,
> + /* Complain */ true, access);
> +
> + }
> + return ExprError(); // Diagnostic is uttered above
> + }
>
> Why not push the diagnostics for overloaded functions into
> PerformContextuallyConvertToBool? Then all clients of
> PerformContextuallyConvertToBool would benefit.
>
>> 3) Fix for a better error message when we can't resolve a function name
>> - When TryImplicitConversion fails in InitializationSequence Constructor
>> before calling it a conversion failure, check to see if it could resolve
>> the name, and if it can't then it is an overloadresolution failure
>> (SemaInit.cpp)
>
> This looks good.
>
>> 4) Fix a bug that allowed invalid deduction during a c-style cast to a
>> function reference
>> - template<class T> void f(T);
>> - (void)((void (&)(int,char))f); // this would compile and should error!!
>> - (void)((void (*)(int,char))f); // error as expected
>> - In 'TryStaticImplicitCast' before allowing a cstyle cast if a
>> simple static cast failed,
>> make sure we are not trying to cstyle cast a 'name' that could not
>> be resolved
>> using the information in the cast (i.e the destination signature)
>> (SemaCXXCast.cpp)
>
> I think this is the code here:
>
> Index: lib/Sema/SemaCXXCast.cpp
> ===================================================================
> --- lib/Sema/SemaCXXCast.cpp (revision 113599)
> +++ lib/Sema/SemaCXXCast.cpp (working copy)
> @@ -962,7 +962,17 @@
> InitializationSequence InitSeq(Self, Entity, InitKind, &SrcExpr, 1);
> if (InitSeq.getKind() == InitializationSequence::FailedSequence &&
> (CStyle || !DestType->isReferenceType()))
> + {
> + // Even a cstyle cast can not convert (or reinterpret) a
> + // name that can not be resolved
> + // do not allow - template<class T> void f(T);
>
> + // (void)((void (&)(int,char))f);
> + if (!(SrcExpr->getType() == Self.Context.OverloadTy &&
> + InitSeq.getFailureKind() ==
> + InitializationSequence::FK_AddressOfOverloadFailed))
> +
> return TC_NotApplicable;
> + }
>
> but this isn't the right approach to a fix. When the implicit cast fails, and
> we have other options (because this is a C-style cast or isn't binding to a
> reference), this function is right to return TC_NotApplicable and the caller
> will try different casts. What's happening with the example code is that
> we're (correctly) returning TC_NotApplicable, but then TryReinterpretCast is
> allowing the bogus cast. We need to fix TryReinterpretCast not to allow it.
>
> Other comments..
>
> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp (revision 113599)
> +++ lib/Sema/SemaExpr.cpp (working copy)
> @@ -4962,7 +4962,17 @@
>
> // cv-unqualified type of the left operand.
> if (PerformImplicitConversion(rExpr, lhsType.getUnqualifiedType(),
> AA_Assigning))
> + {
> + if (rExpr->getType() == Context.OverloadTy)
> + {
> +
> + DeclAccessPair tmp;
> + // Call it again (initially was thru PerformImplict) but now
> Complain
> + ResolveAddressOfOverloadedFunction(rExpr, lhsType
> + , /* Complain */ true, tmp);
> + }
> return Incompatible;
> + }
> return Compatible;
> }
>
> The extra diagnostics for a failed implicit conversion from an overloaded
> function name should be moved into PerformImplicitConversion, rather than
> requiring all of the callers to be updated.
>
> - Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
