[Parser] Avoid correcting delayed typos in array subscript multiple times.

[vsapsai]

Summary

We correct some typos in ActOnArraySubscriptExpr and
ActOnOMPArraySectionExpr, so when their result is ExprError, we can
end up correcting delayed typos in the same expressions again. In
general it is OK but when NumTypos is incorrect, we can hit the
assertion

Assertion failed: (Entry != DelayedTypos.end() && "Failed to get the state for a TypoExpr!"), function getTypoExprState, file clang/lib/Sema/SemaLookup.cpp, line 5219.

This assertion is reproducible with Objective-C method

- (void)test {
  [self undeclaredMethod:undeclaredArg];
  NSMutableDictionary *opts = [NSMutableDictionary new];
  opts[(__bridge id)undeclaredKey] = @0;
}

There is no test for the fix because NumTypos is still incorrect and
we hit the assertion

Assertion failed: (DelayedTypos.empty() && "Uncorrected typos!"), function ~Sema, file clang/lib/Sema/Sema.cpp, line 382.

Another option is to fix tracking delayed typos but it is non-trivial as
in many cases we drop erroneous expressions without cleaning up
TypoExpr they contain. Also the assertion in ~Sema isn't causing
problems with assertions disabled, while a missing TypoExprState can
cause a segmentation fault.

rdar://problem/47403222

[erik.pilkington]

Wait, why is NumTypos incorrect here? I think its because we don't handle the typo on the line: [self undeclaredMethod:undeclaredArg];, even the following asserts now. Seems like the right fix would be to track down why we aren't handling the typo in the message expr.

// RUN: clang -cc1 %s -fobjc-arc
@implementation X
-x { [self undeclaredMethod:undeclaredArg]; }
@end

[vsapsai]

In D60848#1472799, @erik.pilkington wrote:

Wait, why is NumTypos incorrect here? I think its because we don't handle the typo on the line: [self undeclaredMethod:undeclaredArg];, even the following asserts now. Seems like the right fix would be to track down why we aren't handling the typo in the message expr.

// RUN: clang -cc1 %s -fobjc-arc
@implementation X
-x { [self undeclaredMethod:undeclaredArg]; }
@end

The reason why we aren't handling the typo in this case is because TypoExpr is created for undeclaredArg and when we [emit err_arc_may_not_respond](https://github.com/llvm/llvm-project/blob/36371d61ec8ddd13ad922845de6f4c6b95cd21f2/clang/lib/Sema/SemaExprObjC.cpp#L2931-L2934) we just return ExprError and drop MultiExprArg without checking if it contains TypoExpr or not. It is possible to fix this case but we have many more. Some of them are

@class Test;
void test(void) {
  Test *obj;
  [obj test:undeclaredArg];
}

struct S {
  int x;
} S;
void test(void) {
  [S test:undeclaredArg];
}

I think that with the current design for delayed typos it's not feasible to fix all of these lingering delayed typos. Given that with disabled assertions non-empty DelayedTypos in ~Sema isn't causing crashes, I've decided to improve handling the subscripts.

[erik.pilkington]

Huh, okay. I guess there are two separate bugs that are conspiring to crash the compiler here: we shouldn't correct a TypoExpr more than once, and we shouldn't correct a TypoExpr less than once. I think fixing one of the two is fine. FYI I think you should be able to write a testcase if you RUN it with -disable-free.

[vsapsai]

• Add a test case with -disable-free.

Thanks for the suggestion, Erik.

clang/lib/Parse/ParseExpr.cpp

case tok::l_square: {  // postfix-expression: p-e '[' expression ']'
           if (Tok.isNot(tok::r_square))
             Length = ParseExpression();
         }
       } else
         Idx = ParseExpression();
 
       SourceLocation RLoc = Tok.getLocation();
 
-      ExprResult OrigLHS = LHS;
+      LHS = Actions.CorrectDelayedTyposInExpr(LHS);
+      Idx = Actions.CorrectDelayedTyposInExpr(Idx);
+      Length = Actions.CorrectDelayedTyposInExpr(Length);
       if (!LHS.isInvalid() && !Idx.isInvalid() && !Length.isInvalid() &&
           Tok.is(tok::r_square)) {
         if (ColonLoc.isValid()) {
           LHS = Actions.ActOnOMPArraySectionExpr(LHS.get(), Loc, Idx.get(),
                                                  ColonLoc, Length.get(), RLoc);
         } else {
           LHS = Actions.ActOnArraySubscriptExpr(getCurScope(), LHS.get(), Loc,
                                                 Idx.get(), RLoc);
         }
       } else {
         LHS = ExprError();
-      }
-      if (LHS.isInvalid()) {
-        (void)Actions.CorrectDelayedTyposInExpr(OrigLHS);
-        (void)Actions.CorrectDelayedTyposInExpr(Idx);
-        (void)Actions.CorrectDelayedTyposInExpr(Length);
-        LHS = ExprError();
         Idx = ExprError();
       }
 
       // Match the ']'.
       T.consumeClose();
       break;
     }
 

clang/test/SemaCXX/typo-correction.cpp

struct B0 {
   int : 0 |         // expected-error {{invalid operands to binary expression}}
       (struct B0)e; // expected-error {{use of undeclared identifier}}
 };
 
 namespace {
 struct a0is0 {};
 struct b0is0 {};
 int g() {
-  0 [                 // expected-error {{subscripted value is not an array}}
+  0 [
       sizeof(c0is0)]; // expected-error {{use of undeclared identifier}}
 };
 }
 
 namespace avoidRedundantRedefinitionErrors {
 class Class {
   void function(int pid); // expected-note {{'function' declared here}}
 };

clang/test/SemaObjC/typo-correction-subscript.m

+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only -Wno-objc-root-class %s -verify -disable-free
+
+@class Dictionary;
+
+@interface Test
+@end
+@implementation Test
+// rdar://problem/47403222
+- (void)rdar47403222:(Dictionary *)opts {
+  [self undeclaredMethod:undeclaredArg];
+  // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  opts[(__bridge id)undeclaredKey] = 0;
+  // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
+}
+@end