translation of upc_forall missing braces required for correct code about clang-upc HOT 6 CLOSED

clang-upc2c should produce the code that would not cause a warning.

On 3/7/14, 5:19 PM, Paul H. Hargrove wrote:

I've just tried for the first time using the clang built with clang-upc2c as the backend compiler. There are /many/
new "expect pass but got warning" failures in a harness run, and more investigation of those is needed (most are
probably due to the tests themselves). HOWEVER, one at least is the result of the translator.

Given

| upc_forall(int i=0; i<N; i++; i)
if (A[i] != MYTHREAD) errflag=1;
|

the .trans.c contains the following translation of the |upc_forall|:

| if (upcrt_forall_control)
for (int i = 0; i < N; i++)
if ((upcr_get_pshared(&_bupc_spilld0, upcr_add_pshared1(A, 4U, i), 0U, 4U) , _bupc_spilld0) != (int)upcr_mythread())
errflag = 1;
else {
upcrt_forall_control = 1;
for (int i = 0; i < N; i++)
if ((i) % upcr_threads() == upcr_mythread())
if ((upcr_get_pshared(&_bupc_spilld0, upcr_add_pshared1(A, 4U, i), 0U, 4U) , _bupc_spilld0) != (int)upcr_mythread())
errflag = 1;
upcrt_forall_control = 0;
}
|

That is /entirely/ valid C code, but notice that the line immediately before the translator-supplied instance of
|else| in that code is the |if| from the user's code. This results in the /backend/ clang warning:

|foo.trans.c:18:9: warning: add explicit braces to avoid dangling else [-Wdangling-else]
else {
^
1 warning generated.
|

If necessary, I am OK with passing |Wno-dangling-else| to clang as a backend if that is necessary. However, I wanted
to first report the issue here to see if it is reasonable to instead ask/expect/require that clang-upc2c generate
output that clang itself is willing to accept as input. Thoughts?

—
Reply to this email directly or view it on GitHub #44.

from clang-upc.

On 03/07/2014 05:19 PM, Paul H. Hargrove wrote:

That is entirely valid C code, but notice that the line immediately before the translator-supplied instance of else in that code is the if from the user's code. This results in the backend clang warning:

Just because it's valid doesn't mean that it does what you want.
In this case, the warning is correct. The translation needs
to add braces.

from clang-upc.

In this case, the warning is correct.

Ah, right. So the .trans.c code is actually wrong!

The translation needs to add braces.

Is this likely to be a matter of the translation or a problem with StmtPrinter thinking it can omit the braces when the body was a single statement?

from clang-upc.

On 03/07/2014 06:14 PM, Paul H. Hargrove wrote:

The translation needs to add braces.

Is this likely to be a matter of the translation or a problem with StmtPrinter thinking it can omit the braces when the body was a single statement?

It's a problem with the translation. StmtPrinter never
adds braces. The CompoundStmt needs to be represented
in the AST.

from clang-upc.

It's a problem with the translation. StmtPrinter never adds braces.

Well, then I guess I reported the issue against the wrong repo. ;)

from clang-upc.

Fixed in clangupc/upc2c@93b2422

from clang-upc.