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