5

Thanks to Can I implement max(A, max(B, max(C, D))) using fold expressions? I'm aware of one working approach to use std::min in a fold expression(min2 below). However, I'm curious why the below approaches min1 and min3 are considered undefined behavior(seemingly given the warning)?

Per my understanding the expression should evaluate in both cases from left to right, constantly updating myMin and assign the last value back to myMin. Additionally, the final answer is also always correct on both gcc and clang.

template <typename... Args>
auto min1(const Args&... anArgs) {
    constexpr size_t N = sizeof...(anArgs);
    auto myMin = std::get<0>(std::tuple(anArgs...));
    myMin = std::get<N-1>(std::tuple((myMin = std::min(myMin, anArgs))...));
    return myMin;
}

template <typename... Args>
auto min2(const Args&... anArgs) {
    return std::min({anArgs...});
}

template <typename... Args>
auto min3(const Args&... anArgs) {
    auto myMin = (anArgs, ...);
    myMin = ((myMin = std::min(myMin, anArgs)), ...);
    return myMin;
}

The warnings are:

main.cpp: In instantiation of 'auto min1(const Args& ...) [with Args = {int, int, int}]':
main.cpp:26:30:   required from here
main.cpp:8:45: warning: operation on 'myMin' may be undefined [-Wsequence-point]
    8 |     myMin = std::get<N-1>(std::tuple((myMin = std::min(myMin, anArgs))...));
      |                                      ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
main.cpp:8:45: warning: operation on 'myMin' may be undefined [-Wsequence-point]
main.cpp: In instantiation of 'auto min3(const Args& ...) [with Args = {int, int, int}]':
main.cpp:29:30:   required from here
main.cpp:20:10: warning: left operand of comma operator has no effect [-Wunused-value]
   20 |     auto myMin = (anArgs, ...);
      |          ^~~~~
main.cpp:20:10: warning: left operand of comma operator has no effect [-Wunused-value]
main.cpp:21:11: warning: operation on 'myMin' may be undefined [-Wsequence-point]
   21 |     myMin = ((myMin = std::min(myMin, anArgs)), ...);

Coliru Link

Finally, the reason I'm looking into alternate approaches(specifically min1) is because I'm trying to use a 3rd party library for which the comma operator is deprecated and I was wondering if this can still be solved using fold expressions.

eyllanesc
  • 235,170
  • 19
  • 170
  • 241
tangy
  • 3,056
  • 2
  • 25
  • 42

1 Answers1

1

You don't actually have any undefined behavior. Note that the warnings say:

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

(my emphasis)

This warning, in both min1 and min3, comes from the expression

((myMin = std::min(myMin, anArgs))...)

If the parameter pack has 3 elements as it does in your case, this expression will get instantiated as:

((myMin = std::min(myMin, __anArgs0)) , 
 ((myMin = std::min(myMin, __anArgs1)) , 
  (myMin = std::min(myMin, __anArgs2))))

where __anArgs0 etc are just identifiers generated by the implementation. You can see this on cppinsights.

There's no unsequenced operations in this expression that I can see. I'm not really sure why these warnings are being generated, though I'm fairly sure they're false positives.

The expressions between the , are unsequenced, that is true, and hence you get the warning that the entire expression may be undefined. However, since it doesn't matter in which order you compute the 3 different std::min calls, you don't have undefined behavior, and you are always going to get the same result.

The compiler doesn't know this information though, and so it gives you the warning.

You don't get a warning with min2 because the arguments are within a initalizer list, where the order of evaluation of the arguments is well defined.

cigien
  • 57,834
  • 11
  • 73
  • 112
  • 1
    "The expressions between the , are unsequenced" -- not quite: [Rule 9](https://en.cppreference.com/w/cpp/language/eval_order) starting with C++11 guarantees the evaluation order of the comma operator. The most important guarantee is that all side effects of the left hand side expression is sequence before the value computation of the right hand side operator, so you can't evaluate `myMin` in the two or all three calls to `std::min` before applying the side effect of the assignment operator. – Sam Varshavchik Feb 20 '21 at 01:23
  • @SamVarshavchik Hmm, that does seem to be the case. Where is the warning coming from I wonder? Clang shows the same warnings as well. – cigien Feb 20 '21 at 01:27
  • Beats me. I do not see any undefined behavior, based on my read. I think it's a false alarm. – Sam Varshavchik Feb 20 '21 at 01:32
  • @SamVarshavchik Yeah, I can't see any UB either. The fact that both Clang and GCC produce the same warning makes me think there's something deeper going on though, even if it's a false alarm. I should have given it more careful thought before writing up the answer. Oh well, I'll leave it up while I think about it anyway. – cigien Feb 20 '21 at 01:37
  • Well, I could see an argument for the warning in the first parameter pack expansion, as an old warning that does not apply. The first parameter pack expansion is not a folded comma expression, it provides parameters to `std::tuple`'s constructor, and when this is interpreted as a function call pre-C++11 the function call's parameters' side effects are not sequences with each other. Except that std::tuples and parameter packs did not exist until C++11. – Sam Varshavchik Feb 20 '21 at 01:46
  • @SamVarshavchik I would understand a pre-c++11 warning carrying over, but that happening in 2 different compilers seems a lot less likely. Possible, of course. – cigien Feb 20 '21 at 01:52
  • I think it's the extra assignment outside the fold. `x = x = 1;` was undefined before 11. – T.C. Feb 20 '21 at 02:41
  • @T.C. Oh, I didn't know that about pre-11, thanks. Strangely, gcc still [warns](https://godbolt.org/z/d4PfW3) about that. But if that's the explanation, then like the `x=x=1;` case, the warning should point to the assignment outside the fold, right? In this case it points to the inner one. – cigien Feb 20 '21 at 03:15