[Sema] Correct typos in LHS, RHS before building a binop expression.

Specifically, typo correction should be done before dispatching between
different kinds of binary operations like pseudo-object assignment,
overloaded binary operation, etc.

Without this change we hit an assertion

    Assertion failed: (!LHSExpr->hasPlaceholderType(BuiltinType::PseudoObject)), function CheckAssignmentOperands

when in Objective-C we reference a property without `self` and there are
2 equally good typo correction candidates: ivar and a class name. In
this case LHS expression in `BuildBinOp` is

    CXXDependentScopeMemberExpr
    `-TypoExpr

and instead of handling Obj-C property assignment as pseudo-object
assignment, we call `CreateBuiltinBinOp` which corrects typo to

    ObjCPropertyRefExpr '<pseudo-object type>'

but cannot handle pseudo-objects and asserts about it (indirectly,
through `CheckAssignmentOperands`).

rdar://problem/33102722

Reviewers: rsmith, ahatanak, majnemer

Reviewed By: ahatanak

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D37322

llvm-svn: 313323

clang/lib/Sema/SemaExpr.cpp

@@ -11269,6 +11269,26 @@ static NamedDecl *getDeclFromExpr(Expr *E) {
   return nullptr;
 }
 
+static std::pair<ExprResult, ExprResult>
+CorrectDelayedTyposInBinOp(Sema &S, BinaryOperatorKind Opc, Expr *LHSExpr,
+                           Expr *RHSExpr) {
+  ExprResult LHS = LHSExpr, RHS = RHSExpr;
+  if (!S.getLangOpts().CPlusPlus) {
+    // C cannot handle TypoExpr nodes on either side of a binop because it
+    // doesn't handle dependent types properly, so make sure any TypoExprs have
+    // been dealt with before checking the operands.
+    LHS = S.CorrectDelayedTyposInExpr(LHS);
+    RHS = S.CorrectDelayedTyposInExpr(RHS, [Opc, LHS](Expr *E) {
+      if (Opc != BO_Assign)
+        return ExprResult(E);
+      // Avoid correcting the RHS to the same Expr as the LHS.
+      Decl *D = getDeclFromExpr(E);
+      return (D && D == getDeclFromExpr(LHS.get())) ? ExprError() : E;
+    });
+  }
+  return std::make_pair(LHS, RHS);
+}
+
 /// CreateBuiltinBinOp - Creates a new built-in binary operation with
 /// operator @p Opc at location @c TokLoc. This routine only supports
 /// built-in operations; ActOnBinOp handles overloaded operators.
@@ -11301,21 +11321,9 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
   ExprValueKind VK = VK_RValue;
   ExprObjectKind OK = OK_Ordinary;
 
-  if (!getLangOpts().CPlusPlus) {
+  std::tie(LHS, RHS) = CorrectDelayedTyposInBinOp(*this, Opc, LHSExpr, RHSExpr);
-    // C cannot handle TypoExpr nodes on either side of a binop because it
+  if (!LHS.isUsable() || !RHS.isUsable())
-    // doesn't handle dependent types properly, so make sure any TypoExprs have
+    return ExprError();
-    // been dealt with before checking the operands.
-    LHS = CorrectDelayedTyposInExpr(LHSExpr);
-    RHS = CorrectDelayedTyposInExpr(RHSExpr, [Opc, LHS](Expr *E) {
-      if (Opc != BO_Assign)
-        return ExprResult(E);
-      // Avoid correcting the RHS to the same Expr as the LHS.
-      Decl *D = getDeclFromExpr(E);
-      return (D && D == getDeclFromExpr(LHS.get())) ? ExprError() : E;
-    });
-    if (!LHS.isUsable() || !RHS.isUsable())
-      return ExprError();
-  }
 
   if (getLangOpts().OpenCL) {
     QualType LHSTy = LHSExpr->getType();
@@ -11729,6 +11737,13 @@ static ExprResult BuildOverloadedBinOp(Sema &S, Scope *Sc, SourceLocation OpLoc,
 ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
                             BinaryOperatorKind Opc,
                             Expr *LHSExpr, Expr *RHSExpr) {
+  ExprResult LHS, RHS;
+  std::tie(LHS, RHS) = CorrectDelayedTyposInBinOp(*this, Opc, LHSExpr, RHSExpr);
+  if (!LHS.isUsable() || !RHS.isUsable())
+    return ExprError();
+  LHSExpr = LHS.get();
+  RHSExpr = RHS.get();
+
   // We want to end up calling one of checkPseudoObjectAssignment
   // (if the LHS is a pseudo-object), BuildOverloadedBinOp (if
   // both expressions are overloadable or either is type-dependent),

clang/test/SemaObjC/typo-correction.m

@@ -51,3 +51,23 @@ __attribute__ (( __objc_root_class__ ))
 }
 @end
 
+// rdar://problem/33102722
+// Typo correction for a property when it has as correction candidates
+// synthesized ivar and a class name, both at the same edit distance.
+@class TypoCandidate;
+
+__attribute__ (( __objc_root_class__ ))
+@interface PropertyType
+@property int x;
+@end
+
+__attribute__ (( __objc_root_class__ ))
+@interface InterfaceC
+@property(assign) PropertyType *typoCandidate; // expected-note {{'_typoCandidate' declared here}}
+@end
+
+@implementation InterfaceC
+-(void)method {
+  typoCandidate.x = 0; // expected-error {{use of undeclared identifier 'typoCandidate'; did you mean '_typoCandidate'?}}
+}
+@end

clang/test/SemaObjCXX/typo-correction.mm

@@ -36,3 +36,22 @@ 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'}}
 }
+
+// rdar://problem/33102722
+// Typo correction for a property when it has as correction candidates
+// synthesized ivar and a class name, both at the same edit distance.
+@class TypoCandidate;
+
+@interface PropertyType : NSObject
+@property int x;
+@end
+
+@interface InterfaceC : NSObject
+@property(assign) PropertyType *typoCandidate; // expected-note {{'_typoCandidate' declared here}}
+@end
+
+@implementation InterfaceC
+-(void)method {
+  typoCandidate.x = 0; // expected-error {{use of undeclared identifier 'typoCandidate'; did you mean '_typoCandidate'?}}
+}
+@end