1

I encountered a weird issue in the following code snippet, leading to different results.

Apple clang (tested with 3.8.0 and 11.0) returns the expected value 10 but gcc (tested with 5.4.0 and 9) returns 12.

#include <functional>
#include <iostream>
#include <vector>

int acc(std::function<int(int, int)> func, std::vector<int> operands) {
    auto it = operands.begin();
    int result = func(*it, *(++it));  // <-- causing issue, ++it not working as expected
    if (operands.size() > 2) {
        for (++it; it!=operands.end(); ++it) {
            result = func(result, *it);
        }
    }
    return result;
}

int main() {
    std::cout << acc([](int a, int b){ return a+b; }, {3, 5, 2}) << std::endl;
}

It can be reproduced for example, using rextester.com.

While debugging I found out that the iterator increment ++it seems to be the problem. Replacing it by it+1 followed by a it = it + 1 statement leads to the expected result in both compilers. But why is that handled differently among the compilers?

Patrick
  • 1,046
  • 2
  • 10
  • 31
  • 3
    Related (if not a dupe): https://stackoverflow.com/questions/38501587/what-are-the-evaluation-order-guarantees-introduced-by-c17 – Bob__ Apr 30 '20 at 08:09
  • 3
    Note that the code has UB also when the size of the vector is less than 2. You may find useful one of the possible implementations of `std::accumulate`, [here](https://en.cppreference.com/w/cpp/algorithm/accumulate). – Bob__ Apr 30 '20 at 08:18
  • @Bob__ `std::accumulate` takes an initial value, this uses the first element instead – Caleth Apr 30 '20 at 10:08
  • Aside: you could use `std::plus{}` instead of your lambda – Caleth Apr 30 '20 at 10:09

1 Answers1

1

But why is that handled differently among the compilers?

Because before C++17, it was unspecified in what order function arguments are evaluated.

I would suggest not calling func in the initialisation of result.

int acc(std::function<int(int, int)> func, std::vector<int> operands) {
    if (operands.empty()) throw std::out_of_range();

    auto it = operands.begin();
    int result = *it;
    for (++it; it!=operands.end(); ++it) {
        result = func(result, *it);
    }
    return result;
}

Note that std::function is often overkill for passing functions. The standard library prefers templated higher-order functions, e.g. copying std::accumulate

template<class BinaryOperation >
int acc(BinaryOperation op, std::vector<int> operands)

Where BinaryOperation is any function-like type that is callable as int(int, int), such as your lambda's type

Caleth
  • 52,200
  • 2
  • 44
  • 75