dtarditi created this revision.
dtarditi added a reviewer: erik.pilkington.
dtarditi added a subscriber: cfe-commits.
r272587 (http://reviews.llvm.org/D20490) fixed an issue where typo correction
could cause a crash when compiling C programs. The problem was that a typo
expression could be inadvertently processed twice. r272587 fixed this for
BinOp expressions. Conditional expressions can hit the same problem too.
This change adds the two line fix for conditional expressions, as well as a
small test case illustrating the crash.
Note that I don't have commit privileges for clang; someone will need to commit
this for me. I originally started the review by email and am shifting it over
to Phrabicator. The change incorporates the following feedback from Erik
Pilkington on the original patch submitted by email:
1. If we’re doing the check at the end of both branches of the if, we might as
well just move it to after.
2. It doesn’t make sense to have a failure that only occurs in C mode in
test/SemaCXX. Maybe just append it to the end of test/Sema/typo-correction.c.
I see from the prior review that the consensus was that “typo correction is in
a messy state, we should fix this”. I agree. There are other problematic
places in the code where double-processing might or might not occur for C code.
An example is the processing of subscript expressions in
Parser::ParsePostfixExpressionSuffix. Without clear invariants, it is hard to
know what to do.
Testing: clang fails on the test case without this change, passes with it.
The clang test suite results were otherwise the same before and after this
change.
https://reviews.llvm.org/D22930
Files:
lib/Parse/ParseExpr.cpp
test/Sema/typo-correction.c
Index: test/Sema/typo-correction.c
===================================================================
--- test/Sema/typo-correction.c
+++ test/Sema/typo-correction.c
@@ -65,3 +65,18 @@
void fn_with_unknown(int a, int b) {
fn_with_unknown(unknown, unknown | unknown); // expected-error 3 {{use of
undeclared identifier}}
}
+
+// Two typos in a parenthesized expression or argument list with a conditional
+// expression caused a crash in C mode.
+//
+// r272587 fixed a similar bug for binary operations. The same fix was needed
for
+// conditional expressions.
+
+int g(int x, int y) {
+ return x + y;
+}
+
+int h() {
+ g(x, 5 ? z : 0); // expected-error 2 {{use of undeclared identifier}}
+ (x, 5 ? z : 0); // expected-error 2 {{use of undeclared identifier}}
+}
Index: lib/Parse/ParseExpr.cpp
===================================================================
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -446,14 +446,15 @@
LHS = Actions.ActOnBinOp(getCurScope(), OpToken.getLocation(),
OpToken.getKind(), LHS.get(), RHS.get());
- // In this case, ActOnBinOp performed the CorrectDelayedTyposInExpr
check.
- if (!getLangOpts().CPlusPlus)
- continue;
} else {
LHS = Actions.ActOnConditionalOp(OpToken.getLocation(), ColonLoc,
LHS.get(), TernaryMiddle.get(),
RHS.get());
}
+ // In this case, ActOnBinOp or ActOnConditionalOp performed the
+ // CorrectDelayedTyposInExpr check.
+ if (!getLangOpts().CPlusPlus)
+ continue;
}
// Ensure potential typos aren't left undiagnosed.
if (LHS.isInvalid()) {
Index: test/Sema/typo-correction.c
===================================================================
--- test/Sema/typo-correction.c
+++ test/Sema/typo-correction.c
@@ -65,3 +65,18 @@
void fn_with_unknown(int a, int b) {
fn_with_unknown(unknown, unknown | unknown); // expected-error 3 {{use of undeclared identifier}}
}
+
+// Two typos in a parenthesized expression or argument list with a conditional
+// expression caused a crash in C mode.
+//
+// r272587 fixed a similar bug for binary operations. The same fix was needed for
+// conditional expressions.
+
+int g(int x, int y) {
+ return x + y;
+}
+
+int h() {
+ g(x, 5 ? z : 0); // expected-error 2 {{use of undeclared identifier}}
+ (x, 5 ? z : 0); // expected-error 2 {{use of undeclared identifier}}
+}
Index: lib/Parse/ParseExpr.cpp
===================================================================
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -446,14 +446,15 @@
LHS = Actions.ActOnBinOp(getCurScope(), OpToken.getLocation(),
OpToken.getKind(), LHS.get(), RHS.get());
- // In this case, ActOnBinOp performed the CorrectDelayedTyposInExpr check.
- if (!getLangOpts().CPlusPlus)
- continue;
} else {
LHS = Actions.ActOnConditionalOp(OpToken.getLocation(), ColonLoc,
LHS.get(), TernaryMiddle.get(),
RHS.get());
}
+ // In this case, ActOnBinOp or ActOnConditionalOp performed the
+ // CorrectDelayedTyposInExpr check.
+ if (!getLangOpts().CPlusPlus)
+ continue;
}
// Ensure potential typos aren't left undiagnosed.
if (LHS.isInvalid()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
