Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile error with gcc 4.8.2 #142

Closed
patburke opened this issue Jan 26, 2015 · 5 comments
Closed

Compile error with gcc 4.8.2 #142

patburke opened this issue Jan 26, 2015 · 5 comments

Comments

@patburke
Copy link

Using gcc 4.8.2 on Ubuntu 14.04.

src/wren_vm.c: In function ‘runInterpreter’:
src/wren_vm.c:384:41: error: operation on ‘fiber->stackTop’ may be undefined [-Werror=sequence-point]
   #define PUSH(value)  (*fiber->stackTop++ = value)
                                         ^
src/wren_vm.c:562:23: note: in expansion of macro ‘PUSH’
     CASE_CODE(DUP):   PUSH(PEEK()); DISPATCH();
                       ^
@MarcoLizza
Copy link
Contributor

Did notice it too, in the Travis CI output. Quite odd.

@kmarekspartz
Copy link
Contributor

Here's those two macros expanded:

(*fiber->stackTop++ = (*(fiber->stackTop - 1)))

Might it be undefined due to the ++? Perhaps these should be multi-line macros to make the intent clearer / more defined.

@munificent
Copy link
Member

Yeah, my mistake. It's fixed now. :)

For some reason, clang doesn't complain about it, so I didn't catch this locally.

@munificent
Copy link
Member

@zeckalpha I think it might be the =? Either way, the result of the expansion is an expression where there isn't a defined sequence point between accessing and setting stackTop.

@MarcoLizza
Copy link
Contributor

Neither MSC (with the default settings), nor Cppcheck complains about it.

It is due to the post increment operator, @zeckalpha is right.

The evaluation order of the subexpressions is not defined (and mostly compiler dependant), and the post increment side effect could affect the outcome.

(l-value subexpressions are not necessarily evaluated after the r-value ones)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants