[cfe-commits] r125714 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/SemaCXX/member-expr.cpp

[Douglas Gregor]

On Feb 16, 2011, at 10:27 PM, Matt Beaumont-Gay wrote:

> On Wed, Feb 16, 2011 at 19:34, Douglas Gregor <dgregor at apple.com> wrote:
>> 
>> On Feb 16, 2011, at 6:54 PM, Matt Beaumont-Gay wrote:
>> 
>>> Author: matthewbg
>>> Date: Wed Feb 16 20:54:17 2011
>>> New Revision: 125714
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=125714&view=rev
>>> Log:
>>> Fix PR9025 and add a diagnostic (and sometimes a fixit) for an overloaded
>>> function name used as the base of a member expression. Early feedback from
>>> Chandler Carruth, and code review from Nick Lewycky.
>> 
>> Thanks for working on this. A few comments below...
>> 
>>> Modified:
>>>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>    cfe/trunk/lib/Sema/SemaExpr.cpp
>>>    cfe/trunk/test/SemaCXX/member-expr.cpp
>>> 
>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=125714&r1=125713&r2=125714&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Feb 16 20:54:17 2011
>>> @@ -2364,6 +2364,11 @@
>>> def err_typecheck_member_reference_unknown : Error<
>>>   "cannot refer to member %0 in %1 with '%select{.|->}2'">;
>>> def err_member_reference_needs_call : Error<
>>> +  "base of member reference is an overloaded function; perhaps you meant "
>>> +  "to call %select{it|the 0-argument overload}0?">;
>>> +def note_member_ref_possible_intended_overload : Note<
>>> +  "possibly valid overload here">;
>>> +def err_member_reference_needs_call_zero_arg : Error<
>>>   "base of member reference has function type %0; perhaps you meant to call "
>>>   "this function with '()'?">;
>>> def warn_subscript_is_char : Warning<"array subscript is of type 'char'">,
>>> 
>>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=125714&r1=125713&r2=125714&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Feb 16 20:54:17 2011
>>> @@ -3995,6 +3995,8 @@
>>> 
>>>   // There's a possible road to recovery for function types.
>>>   const FunctionType *Fun = 0;
>>> +  SourceLocation ParenInsertionLoc =
>>> +      PP.getLocForEndOfToken(BaseExpr->getLocEnd());
>>> 
>>>   if (const PointerType *Ptr = BaseType->getAs<PointerType>()) {
>>>     if ((Fun = Ptr->getPointeeType()->getAs<FunctionType>())) {
>>> @@ -4029,7 +4031,53 @@
>>>   if (Fun || BaseType == Context.OverloadTy) {
>>>     bool TryCall;
>>>     if (BaseType == Context.OverloadTy) {
>>> -      TryCall = true;
>>> +      // Plunder the overload set for something that would make the member
>>> +      // expression valid.
>>> +      const OverloadExpr *Overloads = cast<OverloadExpr>(BaseExpr);
>>> +      UnresolvedSet<4> CandidateOverloads;
>>> +      bool HasZeroArgCandidateOverload = false;
>>> +      for (OverloadExpr::decls_iterator it = Overloads->decls_begin(),
>>> +           DeclsEnd = Overloads->decls_end(); it != DeclsEnd; ++it) {
>>> +        const FunctionDecl *OverloadDecl = cast<FunctionDecl>(*it);
>>> +        QualType ResultTy = OverloadDecl->getResultType();
>>> +        if ((!IsArrow && ResultTy->isRecordType()) ||
>>> +            (IsArrow && ResultTy->isPointerType() &&
>>> +             ResultTy->getPointeeType()->isRecordType())) {
>>> +          CandidateOverloads.addDecl(*it);
>>> +          if (OverloadDecl->getNumParams() == 0) {
>>> +            HasZeroArgCandidateOverload = true;
>>> +          }
>> 
>> OverloadDecl->getMinRequiredArguments() == 0 would be better here, since it accounts for parameter packs and default arguments.
> 
> Thanks, I'll submit a patch for this tomorrow.
> 
>> 
>> Overall, it would be great to pull the "find a function that we could call with ()" logic out into a subroutine, because there are probably a number of other places where we'd like to have the same kind of recovery.
> 
> Do you have any suggestions about what interface this subroutine
> should have? As a strawman: UnresolvedSet::iterator
> FindZeroArgCall(const UnresolvedSet& Candidates)

That seems reasonable. It might also be nice, if these are non-static member functions, to check that it looks like we can make the call. For example, of the object argument was "const" but the member function was non-const, we'd end up trying to form a bad call.

	- Doug