[PATCH] Fix to PR8880 (clang dies processing a for loop).

[Serge Pavlov]

2013/12/13 Richard Smith <richard at metafoo.co.uk>

>
>   Have you checked that IR generation does the right thing in these cases?
> A CodeGen test for this would be useful.
>
> Attached is a test that can be compiled and run to check particular
binding in clang and gcc.


>
> ================
> Comment at: test/Parser/bad-control.c:22-30
> @@ +21,11 @@
> +
> +// GCC rejects this code
> +void pr8880_3(int first) {
> +  for ( ; ; (void)({ if (first) { first = 0; continue; } 0; })) {}
> +}
> +
> +// GCC rejects this code (see also tests/Analysis/dead-stores.c
> rdar8014335()
> +void pr8880_4(int first) {
> +  for ( ; ; (void)({ if (first) { first = 0; break; } 0; })) {}
> +}
> +
> ----------------
> Serge Pavlov wrote:
> > Richard Smith wrote:
> > > Does this mean we're misinterpreting pr8880_10 and pr8880_11 below?
> > No, this is a feature of GCC. It uses different interpretation depending
> on whether code is compiled as C or as C++. In C++ mode break and continue
> in the second or third expression of for statement refer to the inner loop,
> in C mode - to the outer. However in both modes GCC reject using
> break/continue if for statement is not inside another loop. Clang is more
> consistent, it considers that the third expression refers to the inner loop
> and the second - to outer, the interpretation is the same in C and C++ mode.
> Interesting. I agree that it makes little sense for C and C++ to differ
> here, other than GCC compatibility.
>
> Why does break/continue in the second expression bind to the inner scope
> and the third expression bind to the outer? It seems strange that we would
> (deliberately) differ from both GCC's C mode and its C++ mode here.
>
> Doug Gregor filed a bug against GCC (
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44715), but after 3 years GCC
still keeps inconsistent behavior. Maybe he has more information on this
subject.


> Since GCC rejects this if the loop is non-nested, it seems more reasonable
> to me for them to always bind to the surrounding scope. Is there a reason
> to not do that?
>

In C++ mode break/continue bind to inner scope. Surprisingly this construct
is used in QT macro 'foreach'. The following code:

  foreach(QString name, names)
    qDebug() << name;

after preprocessing produces:

for (QForeachContainer<__typeof__(names)> _container_(names);
     !_container_.brk && _container_.i != _container_.e;
     __extension__ ({ ++_container_.brk; ++_container_.i; }))
for (QString name = *_container_.i;; __extension__ ({--_container_.brk;
break;}))
    qDebug() << name;

So clang must bind break in the third expression to inner loop.

There is also a test in Analysis/dead-stores.c :

// The FOREACH macro in QT uses 'break' statements within statement
expressions
// placed within the increment code of for loops.
void rdar8014335() {
  for (int i = 0 ; i != 10 ; ({ break; })) {
    for ( ; ; ({ ++i; break; })) ;
    // Note that the next value stored to 'i' is never executed
    // because the next statement to be executed is the 'break'
    // in the increment code of the first loop.
    i = i * 3; // expected-warning{{Value stored to 'i' is never read}}
expected-warning{{The left operand to '*' is always 1}}
  }
}

This code cannot be compiled by GCC (I tried 4.6.3 and ToT). The test was
added by Ted Kremenek in r104375. Maybe he can recall if it was obtained
from real application. If so, this is the second usage case clang should
support.



>
> http://llvm-reviews.chandlerc.com/D2018
>



-- 
Thanks,
--Serge
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131213/a777f549/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr8880.c
Type: text/x-csrc
Size: 2115 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131213/a777f549/attachment.c>