[cfe-commits] r59732 - in /cfe/trunk: include/clang/Basic/DiagnosticKinds.def lib/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaOverload.cpp test/SemaCXX/overloaded-operator.cpp

[Doug Gregor]

On Thu, Nov 20, 2008 at 7:44 PM, Chris Lattner <clattner at apple.com> wrote:
> On Nov 20, 2008, at 8:27 AM, Douglas Gregor wrote:
>> +    else
>> +      Diag(OpLoc, diag::err_ovl_no_viable_oper)
>> +        << "operator->" << Base->getSourceRange();
>> +    PrintOverloadCandidates(CandidateSet, /*OnlyViable=*/false);
>> +    delete Base;
>> +    return true;
>
> I think it's awesome you're actually not leaking memory here, we need to do
> more of this in Sema.  As far as patterns go, please using an owning pointer
> to do this.  On entrance to the function, do something like:
>
>  OwningPtr<Expr> Base(BaseInput);
>
> this will trivially delete it on the error paths.

Sure.

>> +  // Build the operator call.
>> +  Expr *FnExpr = new DeclRefExpr(Method, Method->getType(),
>> SourceLocation());
>> +  UsualUnaryConversions(FnExpr);
>> +  Base = new CXXOperatorCallExpr(FnExpr, &Base, 1,
>> +
>> Method->getResultType().getNonReferenceType(),
>> +                                 OpLoc);
>> +  return ActOnMemberReferenceExpr(Base, OpLoc, tok::arrow, MemberLoc,
>> Member);
>
> On the success path, just use ... Base.take() to take ownership from the
> owning ptr

At some point once I'm out there, I'm going to bring up this issue
again, because I feel like the ownership issues in the Parser-Sema
interaction are very unclear, and we are leaking all over the place
when we fail to parser or type-check something. Don't be surprised if
I come back with a manifesto of sorts :)

  - Doug