[clang] Fix ternary operator in the second for loop statement

[danlark]

Summary

Fix ternary operator in for loop argument, it was by mistake not set as CanBeForRangeDecl and led to incorrect codegen. It fixes https://bugs.llvm.org/show_bug.cgi?id=50038. I don't have commit rights. Danila Kutenin. [e-mail]

[lebedev.ri]

This should have codegen, and maybe AST, tests.

clang/test/Parser/cxx2a-init-statement.cpp

18

This isn't cxx2a-specific isn't it?
Isn't this valid even in C99?

[danlark]

In D102502#2759776, @lebedev.ri wrote:

This should have codegen, and maybe AST, tests.

Done

clang/test/Parser/cxx2a-init-statement.cpp

18

clang even in C++11 versions go to this branch and produce warning that this feature is c++2a, so it fails currently for everything but the culprit was https://github.com/llvm/llvm-project/commit/8baa50013c86c34a58d8327c5d1a043898b86398

[danlark]

• Add codegen test

[danlark]

• Add codegen and AST tests

[danlark]

Done, please, take a look

[rsmith]

Thanks, nice catch!

Can we also add an assert when parsing a for statement that we actually find a range if the tentative parse said we were expecting one?

clang/test/CodeGenCXX/for-loop-init-ternary-operator-statement.cpp

1

We don't use -O3 tests for this kind of thing, to avoid depending on the behaviour of the optimizer; instead you should test that the (unoptimized) IR generated by clang is correct.

clang/test/PCH/for-loop-init-ternary-operator-statement.cpp

5

This ast-print output looks wrong; I assume that's an unrelated bug?

[danlark]

• Add codegen and AST tests

clang/test/CodeGenCXX/for-loop-init-ternary-operator-statement.cpp

1

Done.

clang/test/PCH/for-loop-init-ternary-operator-statement.cpp

5

Yes, I think so, in a buggy version x as a second parameter is missing so at least it tests correctly

[danlark]

• Add assert of finding a for range declaration

[danlark]

In D102502#2759900, @rsmith wrote:

Thanks, nice catch!

Can we also add an assert when parsing a for statement that we actually find a range if the tentative parse said we were expecting one?

Done.

[rsmith]

clang/lib/Parse/ParseExprCXX.cpp

2036 ↗

(On Diff #345561)

If parsing failed, I don't think this assertion should be expected to hold: error recovery after the parse error might not have interpreted the : as the start of the range expression.

[danlark]

• Address comment from Richard

[danlark]

Failure looks unrelated, please submit as I don't have commit rights

clang/lib/Parse/ParseTentative.cpp

if (!resolved()) {
       // FIXME: Unify the parsing codepaths for condition variables and
       // simple-declarations so that we don't need to eagerly figure out which
       // kind we have here. (Just parse init-declarators until we reach a
       // semicolon or right paren.)
       RevertingTentativeParsingAction PA(P);
       if (CanBeForRangeDecl) {
         // Skip until we hit a ')', ';', or a ':' with no matching '?'.
         // The final case is a for range declaration, the rest are not.
-        while (true) {
-          unsigned QuestionColonDepth = 0;
+        while (true) {
           P.SkipUntil({tok::r_paren, tok::semi, tok::question, tok::colon},
                       StopBeforeMatch);
           if (P.Tok.is(tok::question))
             ++QuestionColonDepth;
           else if (P.Tok.is(tok::colon)) {
             if (QuestionColonDepth)
               --QuestionColonDepth;
             else {

clang/test/CodeGenCXX/for-loop-init-ternary-operator-statement.cpp

+// RUN: %clang_cc1 -triple i386-unknown-unknown -O3 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define{{.*}} i32 @_Z1fv()
+int f() {
+  // CHECK: ret i32 1
+  for (int i = 0; int x = i < 2 ? 1 : 0; i++) {
+    return x;
+  }
+}

clang/test/PCH/for-loop-init-ternary-operator-statement.cpp

+// RUN: %clang_cc1 -emit-pch -std=c++2a -o %t %s
+// RUN: %clang_cc1 -x ast -ast-print %t | FileCheck %s
+
+int f() {
+  // CHECK: for (int i = 0; x; i++) {
+  for (int i = 0; int x = i < 2 ? 1 : 0; i++) {
+    return x;
+  }
+}
+

clang/test/Parser/cxx2a-init-statement.cpp

 
 void f() {
   int arr1[3];
   for (int n = 5; int x : arr1) {}
 
   int A<0>::*arr2[3];
   for (int n = 5; int A<true ? 0 : 1>::*x : arr2) {}
 
+  for (int i = 0; int x = i < 2 ? 1 : 0; i++) {}
+
   F (*arr3[3])(int);
   for (int n = 5; F (*p)(int n) : arr3) {}
   for (int n = 5; F (*p)(int (n)) : arr3) {}
 
   // Here, we have a declaration rather than an expression.
   for (int n = 5; F (*p)(int (n)); ++n) {}
 
   // We detect whether we have a for-range-declaration before parsing so that