r294008 - [Sema][ObjC++] Typo correction should handle ivars and properties

[Hans Wennborg via cfe-commits]

Thanks! r294059.

On Fri, Feb 3, 2017 at 2:29 PM, Richard Smith <richardsmith at google.com> wrote:
> It looks like the only cases it should have any real impact on are those
> where we would previously miscompile/crash, so this seems OK to me to merge
> to Clang 4.
>
>
> On 3 February 2017 at 13:45, Hans Wennborg <hans at chromium.org> wrote:
>>
>> IIUC, this isn't strictly fixing a regression from 3.9, but it looks
>> like a pretty small diff.
>>
>> Richard, what do you think?
>>
>> On Fri, Feb 3, 2017 at 6:37 AM, Alex L <arphaman at gmail.com> wrote:
>> > Hi Hans,
>> >
>> > Is there any chance we can merge this for 4.0? It fixed a nasty bug
>> > where
>> > clang didn't catch invalid ObjC++ code during semantic analysis which
>> > led to
>> > invalid object files or crashes in CodeGen.
>> >
>> > Cheers,
>> > Alex
>> >
>> > On 3 February 2017 at 14:22, Alex Lorenz via cfe-commits
>> > <cfe-commits at lists.llvm.org> wrote:
>> >>
>> >> Author: arphaman
>> >> Date: Fri Feb  3 08:22:33 2017
>> >> New Revision: 294008
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=294008&view=rev
>> >> Log:
>> >> [Sema][ObjC++] Typo correction should handle ivars and properties
>> >>
>> >> After r260016 and r260017 disabled typo correction for ivars and
>> >> properties
>> >> clang didn't report errors about unresolved identifier in the base of
>> >> ivar
>> >> and
>> >> property ref expressions. This meant that clang invoked CodeGen on
>> >> invalid
>> >> AST
>> >> which then caused a crash.
>> >>
>> >> This commit re-enables typo correction for ivars and properites, and
>> >> fixes
>> >> the
>> >> PR25113 & PR26486 (that were originally fixed in r260017 and r260016)
>> >> in a
>> >> different manner by transforming the Objective-C ivar reference
>> >> expression
>> >> with
>> >> 'IsFreeIvar' preserved.
>> >>
>> >> rdar://30310772
>> >>
>> >> Modified:
>> >>     cfe/trunk/lib/Sema/SemaExprCXX.cpp
>> >>     cfe/trunk/lib/Sema/TreeTransform.h
>> >>     cfe/trunk/test/SemaObjCXX/typo-correction.mm
>> >>
>> >> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
>> >> URL:
>> >>
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=294008&r1=294007&r2=294008&view=diff
>> >>
>> >>
>> >> ==============================================================================
>> >> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
>> >> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Feb  3 08:22:33 2017
>> >> @@ -7250,14 +7250,6 @@ public:
>> >>
>> >>    ExprResult TransformBlockExpr(BlockExpr *E) { return Owned(E); }
>> >>
>> >> -  ExprResult TransformObjCPropertyRefExpr(ObjCPropertyRefExpr *E) {
>> >> -    return Owned(E);
>> >> -  }
>> >> -
>> >> -  ExprResult TransformObjCIvarRefExpr(ObjCIvarRefExpr *E) {
>> >> -    return Owned(E);
>> >> -  }
>> >> -
>> >>    ExprResult Transform(Expr *E) {
>> >>      ExprResult Res;
>> >>      while (true) {
>> >>
>> >> Modified: cfe/trunk/lib/Sema/TreeTransform.h
>> >> URL:
>> >>
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=294008&r1=294007&r2=294008&view=diff
>> >>
>> >>
>> >> ==============================================================================
>> >> --- cfe/trunk/lib/Sema/TreeTransform.h (original)
>> >> +++ cfe/trunk/lib/Sema/TreeTransform.h Fri Feb  3 08:22:33 2017
>> >> @@ -2982,16 +2982,17 @@ public:
>> >>    ExprResult RebuildObjCIvarRefExpr(Expr *BaseArg, ObjCIvarDecl *Ivar,
>> >>                                            SourceLocation IvarLoc,
>> >>                                            bool IsArrow, bool
>> >> IsFreeIvar)
>> >> {
>> >> -    // FIXME: We lose track of the IsFreeIvar bit.
>> >>      CXXScopeSpec SS;
>> >>      DeclarationNameInfo NameInfo(Ivar->getDeclName(), IvarLoc);
>> >> -    return getSema().BuildMemberReferenceExpr(BaseArg,
>> >> BaseArg->getType(),
>> >> -                                              /*FIXME:*/IvarLoc,
>> >> IsArrow,
>> >> -                                              SS, SourceLocation(),
>> >> -
>> >> /*FirstQualifierInScope=*/nullptr,
>> >> -                                              NameInfo,
>> >> -
>> >> /*TemplateArgs=*/nullptr,
>> >> -                                              /*S=*/nullptr);
>> >> +    ExprResult Result = getSema().BuildMemberReferenceExpr(
>> >> +        BaseArg, BaseArg->getType(),
>> >> +        /*FIXME:*/ IvarLoc, IsArrow, SS, SourceLocation(),
>> >> +        /*FirstQualifierInScope=*/nullptr, NameInfo,
>> >> +        /*TemplateArgs=*/nullptr,
>> >> +        /*S=*/nullptr);
>> >> +    if (IsFreeIvar && Result.isUsable())
>> >> +      cast<ObjCIvarRefExpr>(Result.get())->setIsFreeIvar(IsFreeIvar);
>> >> +    return Result;
>> >>    }
>> >>
>> >>    /// \brief Build a new Objective-C property reference expression.
>> >>
>> >> Modified: cfe/trunk/test/SemaObjCXX/typo-correction.mm
>> >> URL:
>> >>
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/typo-correction.mm?rev=294008&r1=294007&r2=294008&view=diff
>> >>
>> >>
>> >> ==============================================================================
>> >> --- cfe/trunk/test/SemaObjCXX/typo-correction.mm (original)
>> >> +++ cfe/trunk/test/SemaObjCXX/typo-correction.mm Fri Feb  3 08:22:33
>> >> 2017
>> >> @@ -21,3 +21,18 @@ public:
>> >>    self.m_prop2 = new ClassB(m_prop1); // expected-error {{use of
>> >> undeclared identifier 'm_prop1'; did you mean '_m_prop1'?}}
>> >>  }
>> >>  @end
>> >> +
>> >> +// rdar://30310772
>> >> +
>> >> + at interface InvalidNameInIvarAndPropertyBase
>> >> +{
>> >> + at public
>> >> +  float  _a;
>> >> +}
>> >> + at property float _b;
>> >> + at end
>> >> +
>> >> +void invalidNameInIvarAndPropertyBase() {
>> >> +  float a = ((InvalidNameInIvarAndPropertyBase*)node)->_a; //
>> >> expected-error {{use of undeclared identifier 'node'}}
>> >> +  float b = ((InvalidNameInIvarAndPropertyBase*)node)._b; //
>> >> expected-error {{use of undeclared identifier 'node'}}
>> >> +}
>> >>
>> >>
>> >> _______________________________________________
>> >> cfe-commits mailing list
>> >> cfe-commits at lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >
>> >
>
>