-1

In the legacy code base I'm working on, I discovered the line

n = ++n % size;

that is just a bad phrasing of the intended

n = (n+1) % size;

as deduced from the surrounding code and runtime-proved. (The latter now replaces the former.)

But since this code was marked as an error by Cppckeck, and caused a warning in GCC, without ever having caused any malfunction, I didn't stop thinking here. I reduced the line to

n = ++n;

still getting the original error/warning messages:

Cppcheck 1.80:

Id: unknownEvaluationOrder
Summary: Expression 'n=++n' depends on order of evaluation of side effects
Message: Expression 'n=++n' depends on order of evaluation of side effects

GCC (mingw32-g++.exe, version 4.9.2, C++98):

warning: operation on 'n' may be undefined [-Wsequence-point]|

I already learned that assignment expressions in C/C++ can be heavily affected by undefined evaluation order, but in this very case I just can't imagine how.

Can the undefined evaluation order of n = ++n; really be relevant for the resulting program, especially for intended value of n? That's what I imagine what may happen.

Scenario #1
++n;
n=n;

Scenario #2
n=n;
++n;

I know that the meaning and implications of relaying on undefined behaviour in C++, is hard to understand and hard to teach.

I know that the behaviour of n=++n; is undefined by C++ standards before C++11. But it has a defined behaviour from C++11 on, and this (now standard-defined behaviour) is exactly the same I'm observing with several compilers[1] for this small demo program

#include <iostream>

using namespace std;

int main()
{
    int n = 0;
    cout << "n before: " << n << endl;
    n=++n;
    cout << "n after: " << n << endl;
    return 0;
}

that has the output

n before: 0
n after: 1

Is it reasonable to expect that the behaviour is actually the same for all compilers regardless of being defined or not by standards? Can you (a) show one counter example or (b) give an easy to understand explanation how this code could produce wrong results?


[1] the compilers a used

Wolf
  • 9,679
  • 7
  • 62
  • 108
  • 1
    An expression has undefined behaviour if it updates any variable more than once. `n = ++n` updates `n` both during the increment, and during the assignment, hence undefined behaviour (the fact that both updates would produce the same value doesn't change that). `n = (n+1) % size` is well-defined, since it only updates `n` once. – Peter Sep 06 '17 at 09:54
  • "which I found out (looking around) to be just a very confusing spelling of `n = (n+1) % size;`" - you should have done more research. That's so very wrong. – too honest for this site Sep 06 '17 at 10:10
  • @Olaf as far as I see, [there is postincrement](https://stackoverflow.com/q/949433/2932052) being used. – Wolf Sep 06 '17 at 10:10
  • @Olaf *`That's so very wrong`* -- I looked in the surrounding code to detect the intended meaning, maybe I should reword that in the question. – Wolf Sep 06 '17 at 10:12
  • @Wolf: Yes and there might be different whitespaces, too. So what? Get a C or C++ book. Btw: Tag for the language you use. C and C++ are **different** languages (it just happens these constructs are UB in both). (and please don't use code-markdown when citing non-code) – too honest for this site Sep 06 '17 at 10:13
  • 2
    @Olaf Why are you so aggressive? Wolf's comment is legitimate because the pre and post increment operators do not follow the same rules since c++11, proof is that `n = ++n + 2;` seems to be well-defined while `n = n++ + 2;` is not. Moreover, the dupe is targeted for `C`, maybe the rule are different between C11 and C++11, but in any cases the dupe is probably outdated, and will likely be even more with c++17 coming soon. – Holt Sep 06 '17 at 10:20
  • @Olaf I dont see the duplication, I'm asking about the relevance of "undefined evaluation order", not just about UB. – Wolf Sep 06 '17 at 10:22
  • @Holt: 1) A straight comment is not aggressive. I consider such allegations as rude. 2) The tag-wiki should clear this. 3) Without further proof in the question, I tend to respect the compiler's warning which supports it is still UB. But feel free to provide an answer proving me wrong. There are still more CVs needed. – too honest for this site Sep 06 '17 at 10:27
  • @Olaf Well, please excuse me, I withdraw my allegations, but for non-native speakers (which I am), the first part of your comments seemed pretty rude. I agree with you that a citation from cppreference is not enough to clarify this, I'll try finding the relevant standard quotes before posting an answer here. Regarding the tag wiki, I think that a new community question should be posted and answered, specifically for C++, with the old and new rules regarding these. – Holt Sep 06 '17 at 10:32
  • @BoPersson I do understand this, I don't say this question is very different from others, I am saying that I think the tag wiki may not be appropriate anymore (and thus the related dupe), and that something should be done about it, but maybe this is not the place to talk about that. – Holt Sep 06 '17 at 10:34
  • @Holt: Askers are assumed to understand the subject they ask about, resp. to do research about comments/links. If not, they are free to ask another question (recursive). If they insist on their position although they got a clear proof they are wrong is typically a signal there was no research (expecially if the reply comes that quick), The problem is such questions still get (more or less bad) answers which could keep them from being automatically deleted once closed (keyword: "roomba"), increasing work-load for site-cleaners of which I am one (nothing special, just a mater of reputation). – too honest for this site Sep 06 '17 at 10:36
  • @holt: If there really has changed something with the C++11 standard version (and not just something like "any reasonable compiler generates the expected code"), how about a self-answered cannonocal question specifically for C++ (it definitively has not changed with C11). Just make it clear in the question and post the answer quickly. Then edit the wiki page and link to your question. If done well, I'd be happy to upvote both or comment what I miss, Q&A (just pink me, I don't monitor the C++ tag). – too honest for this site Sep 06 '17 at 10:52
  • @Olaf please have a look again, if you consider it still a duplicate. It's about understanding running code. Thanks – Wolf Sep 06 '17 at 11:01
  • @Wolf: I changed the tag to C now, as you state it is C code. Would you **please** 1) Notice C and C++ are difererent languages. and 2) Read and understand the dupes and why it still is a dupe. and 3) learn what **undefined behaviour** implies? "It ran for decades" does tell exactly _zero_ about the code being correct. Old legacy compilers were way more straight forward with parser/code-generator. Any modern compiler might do anything with that stuff. If you mean `n =n + 1 % x`, **just write it**, this would be fine! – too honest for this site Sep 06 '17 at 11:13
  • (Heck, I'm seriously tired of these "but it worked" whinings. What's the problem doing it right?? All that argumenting takes way more time of your's (worse: mine) than that simple edit of the code. – too honest for this site Sep 06 '17 at 11:16
  • @Olaf if you are preparing porting code it to newer compilers, what would you do: rewrite all? I just try to understand error patterns in real code that I *have* to deal with. And retagging it to C because my additions does not really make sense. It's C++. Please have another look. Sorry, but it's tiring for me too – Wolf Sep 06 '17 at 11:25
  • @BoPersson sorry for taking your time again, I only wish to hear a second voice about the dup aspect after reworking the question. My question asks about "working" code using the pre-increment operator (I'm definitely not interested in building some artificial UB question). Would you please have a look on it? Thanks – Wolf Sep 06 '17 at 11:39
  • @Wolf: Last post: I definitively would clean the mess up and fix the UB (note there are certain kinds of UB which are well defined by the implementation, but this one is not). If the customer does not accept this, I't drop the project. And I'm on embedded systems where I have much more deterministic implementations. I laready told you how to deal with that sh**t. If you were in one of my projects, even asking such things, we had a **single** serious talk. If you still were complaining, you better would change teams or companies. Seriously, all this took way to much resources already. – too honest for this site Sep 06 '17 at 11:41
  • @BoPersson: It has been broken for decades, that's for sure and I'd be even more careful to compile legacy code as C++11 or newer, there are quite likely even worse problems. Are you sure, it is now well-defined? In that case we inded need a new cannonical Q&A. I still don't see how this can be we--defined, expecially considering the more advanced features in the newer versions of C++. But I'm not that much of a C++ guy, so I'm fine being **proved** I'm wrong. – too honest for this site Sep 06 '17 at 11:55
  • @Olaf - My last sentence wasn't supposed to mean that it works now, but that I would fix the code anyway, *even if it now did work*. Because I cannot keep track of which standard release possibly added some partial ordering of evaluation for these expressions. And they are not very useful anyway. – Bo Persson Sep 06 '17 at 12:10
  • @BoPersson: Well, I'm currently thinking to dive again into C++, concentrating on the modern (i.e. since C++11) version(s), as they changed a lot. So I'm really interested if such constructs are indeed valid now. And it would be good to have a distinct answer covering this here, too. (Least it would be another point on the list of differences between C and C++). – too honest for this site Sep 06 '17 at 12:17
  • @Olaf - If you sort this out, I expect comments like *"This should only work for C++17 but I tried it with C++14 and it worked there too. Is the answer wrong?"* But go ahead... :-) – Bo Persson Sep 06 '17 at 12:24
  • A C++98-compliant compiler is actually used, retagged the question correspondingly, sorry for the fus. – Wolf Sep 06 '17 at 12:24
  • I reworked the question as to make it better answerable and think it's now better to see that it is not the duplicate of the questions listed. I read the quoted questions the corresponding answers and did not find any attempt to explain why (or *how*) `i=++i;` could fail. Many of them cite that `i=++i+1;` which seems similar but also here no explanations what the problem behind may look like... That's why I vote to reopen my question. – Wolf Sep 08 '17 at 09:46
  • You have spent far more time posting this query than replacing the code with the clear, concise, correct, and unambiguous `n = (n+1) % size`. What's the point? Why would you even *want* to write `n = ++n`? What are you trying to say (to the compiler, to other human readers of the code) that you couldn't say more simply and conventionally? Even if C++11 gives this well-defined behavior, why depend on it? Do you enjoy going up to sleeping fire-breathing dragons and poking them with sharp sticks, too? – Steve Summit Sep 08 '17 at 21:05
  • @SteveSummit the only purpose of this question is to show a sort of "working" code that *should be touched*. I just was the one who read this n=++n... bullshit - Let me assure you: I'm the last person who appreciates this kind of code. I really hoped for someone on SO who was able to show *how* even this code could be compiled to something not working. – Wolf Sep 11 '17 at 11:30

2 Answers2

3

The increment order is precisely defined. It is stated there that

i = ++i + 2;       // undefined behavior until C++11

Since you use a C++11 compiler, you can leave your code as is is. Nevertheless, I think that the expressiveness of

n = (n+1) % size;

is higher. You can more easily figure out what was intended by the writer of this code.

schorsch312
  • 5,553
  • 5
  • 28
  • 57
  • *Since you use a C++11 compiler* -- to be honest the C++11 setting in GCC was by chance, the code is something that tries to be C++98 compliant. I just tried to figure out how the order of assignment and postincrement **could** be combined to produce wrong results. (The code does it's job perfectly for years.) – Wolf Sep 06 '17 at 10:36
  • 1
    And I have never understood why `i = ++i + 2;` is so interesting when you can write `i += 3;`. :-) – Bo Persson Sep 06 '17 at 10:45
  • @BoPersson n++ is incrementing n by 1, so ((n++)++)++ is perhaps the most nerdy way to code it :-) – schorsch312 Sep 06 '17 at 10:47
  • Sorry, after rewording my question, I have to confess that your answer doesn't really *answer* it. But its it's definitely useful, but that I already stated the SO-way. :-) Maybe it's the difference (or relation) between undefined behaviour and undefined evaluation order that confuses me in that case... – Wolf Sep 06 '17 at 11:45
  • The answer is wrong. OP does not use modern C++, but - according to his comment, C++98. Further more, cppreference is not an authoritative resource. For questions targeting the standard, please reference the standard. And trhis question is a dupe and an explicit no-ask question and should not be answered, but closed. – too honest for this site Sep 06 '17 at 13:34
-1

According to cppreference:

If a side effect on a scalar object is unsequenced relative to another side effect on the same scalar object, the behavior is undefined:

i = ++i + 2;       // undefined behavior until C++11
i = i++ + 2;       // undefined behavior until C++17
f(i = -2, i = -2); // undefined behavior until C++17
f(++i, ++i);       // undefined behavior until C++17, unspecified after C++17
i = ++i + i++;     // undefined behavior

For the case n = ++n; it would be an undefined behavior but we do not care which assignment happens first, n = or ++n.

Griffin
  • 716
  • 7
  • 25
  • So you'd say the resulting `n` would be the original `n` incremented by one? Seems, the only problem could be a slightly lower performance. But I don't see the *two* unsequenced side effects here. – Wolf Sep 06 '17 at 10:08
  • This question is explicitly listed as do-not-ask for both of the different languages, C and C++, see the wiki. Don't answer such questions, but vote/flag as dube. – too honest for this site Sep 06 '17 at 10:11
  • `but we do not care which assignment happens first`; Can you explain why? – Stefan Sep 06 '17 at 10:17
  • @Wolf if it is C, ask specifically about C, but then the only valid answer is it's UB (and there are tons of duplicates) ... –  Sep 06 '17 at 10:18
  • @Olaf The PO has changed the tag. I do not understand your reasons for such questions not to be answered and down-voted. – Griffin Sep 06 '17 at 10:19
  • 1
    @Stefan Because whether the first assignment `n = ` happens first or the incremental assignment `++n` happens first the final value of `n` in the case of `n = ++n;` is the same. – Griffin Sep 06 '17 at 10:19
  • @Griffin: it's explicitly stated in the tag's wiki: https://stackoverflow.com/tags/c%2b%2b/info – Stefan Sep 06 '17 at 10:20
  • @Griffin: It was me removing the unrelated tags. And will you please just read the tag-wiki? That question is asked multiple times a day in one form or the other. Btw. your answer is missleading, if not wrong due to the last sentence. – too honest for this site Sep 06 '17 at 10:21
  • @Olaf To use the wrong tag is good to be pointed out to be fixed. If duplicate, flag it as duplicate with link to the answered question. If my answer is misleading correct with constructive comments. – Griffin Sep 06 '17 at 10:23
  • I still don't get it; if `i = ++i + 2;` is undefined, I assume `i = ++i + 0;` is too. This would be the equivalent (I would guess) of `i = ++i;`. Can you explain why it's different? – Stefan Sep 06 '17 at 10:31
  • @Griffin: I did point out what makes your answer "problematic". For the do-not-answer: that is a well meant comment, as under certain conditions, such answers will prevent the question from roomba. So thank you for increasing work-load for those people who care about quality. – too honest for this site Sep 06 '17 at 10:32
  • @Stefanq: Of course it is! That's the point why the question indeed is dupe #12233479954218833 of the daily mantra. – too honest for this site Sep 06 '17 at 10:33
  • As a sidenote: cppreference is not an athoritative resource. This is a standard question, so please cite the standard, which is very clear. – too honest for this site Sep 06 '17 at 11:46