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

C / CPP backends differ in argument evaluation order #8202

Closed
ConsoleTVs opened this issue Jul 3, 2018 · 13 comments

Comments

Projects
None yet
5 participants
@ConsoleTVs
Copy link

commented Jul 3, 2018

More info: https://forum.nim-lang.org/t/4010

This is a serious issue...

How I fixed it for me: ConsoleTVs/TFG@c3698f6#diff-ecd2a08316dc75e0142a9547a218c408

EDIT: added code to repreduce:

var current: int = 0

proc gen(): string = current.inc; $(current - 1)

proc allOut(a, b, c: string) =
    echo a
    echo b
    echo c

allOut(gen(), gen(), gen())
@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Jul 4, 2018

Please, can you add details here on how to reproduce the issue?

@ConsoleTVs

This comment has been minimized.

Copy link
Author

commented Jul 4, 2018

The issue itself seems to evaluate the arguments of functions in a different order when using c++ backend.

In my case. As the fixed link i gave, you can see how to fix it by forcing to evaluate the parameters outside the argument list. Somebosy answered that on the forum, it might help you out

@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Jul 4, 2018

Yeah, that is clear, I was asking if you could post a reproducible example here that does not involve external resources and repositories

@ConsoleTVs

This comment has been minimized.

Copy link
Author

commented Jul 4, 2018

Yes sure:

var current: int = 0

proc gen(): string = current.inc; $(current - 1)

proc allOut(a, b, c: string) =
    echo a
    echo b
    echo c

allOut(gen(), gen(), gen())

This will give:

c:

0
1
2

cpp:

2
1
0
@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Jul 4, 2018

Thank you!

@stefanos82

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

I can confirm that indeed the C++ generated output is indeed as above, whereas C produces the expected output.

Nice catch @ConsoleTVs 👍

@ConsoleTVs

This comment has been minimized.

Copy link
Author

commented Jul 4, 2018

@stefanos82 hehe, It was discovered while developing a stack based VM, where the order of POP instructions matter :3 Still kinda fixable by using a variable asign and then pass the arguments. The problem relies in evaluation order of arguments :)

Thanks tho!

@ConsoleTVs ConsoleTVs changed the title [IMPORTANT] C / CPP backend types differ in argument evaluation [IMPORTANT] C / CPP backends differ in argument evaluation order Jul 4, 2018

@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Jul 5, 2018

I wonder if one could make a forceOrderOfEvaluation macro that automatically does the rewrite using temporaries for all function calls in its body

@Araq

This comment has been minimized.

Copy link
Member

commented Jul 5, 2018

@andreaferretti That's entirely possible, but it's just a C++ codegen bug and shouldn't be hard to fix either.

@krux02

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2018

Well I looked at the generated cpp code and the generated c code:

c:

	T1_ = (NimStringDesc*)0;
	T1_ = gen_kR9bLIeU6XIHDD3f3654Jsg();
	T2_ = (NimStringDesc*)0;
	T2_ = gen_kR9bLIeU6XIHDD3f3654Jsg();
	T3_ = (NimStringDesc*)0;
	T3_ = gen_kR9bLIeU6XIHDD3f3654Jsg();
	allOut_b3bLY9bg2hfU2vLNvWPXntw(T1_, T2_, T3_);

cpp

	allOut_b3bLY9bg2hfU2vLNvWPXntw(gen_kR9bLIeU6XIHDD3f3654Jsg(), gen_kR9bLIeU6XIHDD3f3654Jsg(), gen_kR9bLIeU6XIHDD3f3654Jsg());

My personal opinion is that it is weird that they differ this much. But apart from that I think both are correct. I think no program should rely on the order of argument evaluation. Is the order of argument evaluation specified in Nim? Because in neither C nor Cpp it is specified. On a different C++ compiler you might get a different order of argument evaluation. I know this and I never had problems with this.

@Araq

This comment has been minimized.

Copy link
Member

commented Jul 9, 2018

Is the order of argument evaluation specified in Nim? Because in neither C nor Cpp it is specified.

It is not yet written down, but the order of evaluation is left-to-right and the C++ code needs to introduce temporaries just like the C version does.

@ConsoleTVs

This comment has been minimized.

Copy link
Author

commented Jul 9, 2018

@krux02 @Araq

I agree with @Araq , The order is left to right. Yet even if diferent programing languages like c / cpp / js differ in argument evaluation or other things, the nim compiler should ALWAYS make sure the entire program does exactly the same regardless of the backend.

@krux02

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2018

@ConsoleTVs As I said it is not just that different programming languages differ in argument evaluation, it is that different compilers differ in argument evaluation order. Try it with different backend like visual studio compiler and gcc. So I wouldn't argument that this issue is [IMPORTANT]. It wouldn't be a surprise to any programmer who comes from c or c++ that you should not rely on the order of argument evaluation. Not just because compilers might change the order at will, but also because of readability.

@krux02 krux02 changed the title [IMPORTANT] C / CPP backends differ in argument evaluation order C / CPP backends differ in argument evaluation order Oct 25, 2018

Araq added a commit that referenced this issue Mar 22, 2019

@Araq Araq closed this in 0cc6e12 Mar 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.