The following code: #include <utility> struct Do_Call { template<typename Func, typename ... T> Do_Call(const Func &f, T && ... t) { f(std::forward<T>(t)...); } }; int main() { const auto sum = [](int w, int x, int y, int z) { return w + x + y + z; }; int i = 0; Do_Call{sum, ++i, ++i, ++i, ++i}; } Generates the -Wsequence-point warning for the variable 'i'. However, the standard states: "Within the initializer-list of a braced-init-list, the initializer-clauses, including any that result from pack expansions (14.5.3), are evaluated in the order in which they appear. That is, every value computation and side effect associated with a given initializer-clause is sequenced before every value computation and side effect associated with any initializer-clause that follows it in the comma-separated list of the initializer-list. [Note: This evaluation ordering holds regardless of the semantics of the initialization; for example, it applies when the elements of the initializer-list are interpreted as arguments of a constructor call, even though ordinarily there are no sequencing constraints on the arguments of a call. —end note ]" The "Note: This evaluation ordering holds regardless of the semantics of the initialization; for example, it applies when the elements of the initializer-list are interpreted as arguments of a constructor call, even though ordinarily there are no sequencing constraints on the arguments of a call. —end note" is the most applicable to this bug report. This occurs in all versions tested between 4.7.3->6
Note: I tested this code further and the compiler seems to be generating incorrect code based on this standard, not just warning incorrectly.
Follow up, this is a better test case that does not pass by && struct MyType { MyType(int i, int j, int k, int l) : sum(i + j + k + l) { } int sum; }; int main() { int i = 0; std::cout << MyType{ ++i, ++i, ++i, ++i }.sum << '\n'; }
Confirmed that g++ prints 3 -Wsequence-point warnings on each testcase. (the followup one is missing an include for <iostream>, but after that, it behaves as reported)
It seems bug 51253 previously addressed this, which means this is a regression. Also bug 65866 exists for the warning itself. I can confirm incorrect code generation using g++ 8.2.0 on this simpler testcase: #include <utility> int main() { int i = -1; return std::pair{ ++i, ++i }.first; } $ g++ -Wall -std=c++17 -o list-init-sequence{,.cc} list-init-sequence.cc: In function ‘int main()’: list-init-sequence.cc:6:20: warning: operation on ‘i’ may be undefined [-Wsequence-point] return std::pair{ ++i, ++i }.first; ^~~ $ ./list-init-sequence || echo fail fail Interestingly, this variation does not produce a warning but still produces incorrect code: #include <utility> int main() { int i = 0; return std::pair{ i, ++i }.first; } $ g++ -Wall -std=c++17 -o list-init-sequence{,.cc} $ ./list-init-sequence || echo fail fail
(In reply to Matthijs van Duin from comment #4) > It seems bug 51253 previously addressed this, which means this is a > regression. No. To be a regression it has to have previously worked, then stopped working. All versions of GCC warn about this code, so it's not a regression. The fix for bug 51253 only changed the code generation, it didn't stop the warnings being printed.
(In reply to Jonathan Wakely from comment #5) > The fix for bug 51253 only changed the code generation, it didn't stop the > warnings being printed. And it didn't affect the result for any of the testcases in this bug. So it seems the fix for 51253 was simply incomplete, but there's no regression.
Ah! I should have checked the actual tests of 51253 before calling it a regression, apologies. I just kinda assumed that these simple cases would be covered by the work done back then.
(In reply to Matthijs van Duin from comment #4) > return std::pair{ ++i, ++i }.first; My bad! This isn't an exhibit of the bug. I simply forgot that std::pair is not really a struct, and this isn't aggregate initialization: the constructor takes references, so correct code is generated in this case. And in fact, if you do use an aggregate, the test works correctly. However, if you replace std::pair by a class whose constructor takes (int, int), similar to the one used in the existing testcase (g++.dg/cpp0x/initlist86.C) then it fails again. Looking at the disassembly (on ARM since I don't know x86 asm) shows that gcc loads both arguments from the storage allocated for i, after both increments have been done. Effectively it's copy-constructing the first argument too late. The more general issue appears to be that if the arguments are trivially copyable lvalues, then gcc keeps these as lvalues and copy-constructs the actual arguments way too late. If I look at this disassembly of this code: struct Foo { char x[64]; // too big to pass in register Foo( Foo const &other ) = default; // but still trivially copyable Foo &mutate(); }; struct Pair { Pair( Foo x, Foo y ); }; void test( Foo &foo ) { Pair{ foo.mutate(), foo.mutate() }; } Then test() effectively does: Foo &temp1 = foo.mutate(); Foo &temp2 = foo.mutate(); Pair{ temp1, temp2 } // copy-construct arguments and call Pair constructor (Also, interestingly, temp2 is copy-constructed before temp1 is!) If Foo is not trivially copyable, even if merely due to the presence of a destructor, then the problem disappears.
(In reply to Matthijs van Duin from comment #4) > Also bug 65866 exists for the warning itself. I think this is an exact dup. *** This bug has been marked as a duplicate of bug 65866 ***
(In reply to Jonathan Wakely from comment #9) > (In reply to Matthijs van Duin from comment #4) > > Also bug 65866 exists for the warning itself. > > I think this is an exact dup. > > *** This bug has been marked as a duplicate of bug 65866 *** Well, this bug was amended to be about incorrect code generation (although the reporter did not update the title), while bug 65866 is purely about incorrect diagnostics. I'm assuming this means a new bug should be opened about the wrong code generation? (assuming one hasn't been created for it yet)
(In reply to Matthijs van Duin from comment #10) > I'm assuming this means a new bug should be opened > about the wrong code generation? Yes please. This one is too confusing now.
(In reply to Jonathan Wakely from comment #11) > (In reply to Matthijs van Duin from comment #10) > > I'm assuming this means a new bug should be opened > > about the wrong code generation? > > Yes please. This one is too confusing now. Found an existing one, bug 70796.
Thanks!