Niklas R
Niklas R

Reputation: 16860

c++11 parameter pack wrong behaviour with Apple LLVM 7.0.0 but works with GCC-5.1

Going from a previous version of this question, thanks to @Gene, I was now able to reproduce this behaviour using a simpler example.

#include <iostream>
#include <vector>

class Wrapper
{
  std::vector<int> const& bc;
public:
  Wrapper(std::vector<int> const& bc) : bc(bc) { }
  int GetSize() const { return bc.size(); }
};

class Adapter
{
  Wrapper wrapper;
public:
  Adapter(Wrapper&& w) : wrapper(w) { }
  int GetSize() const { return wrapper.GetSize(); }
};

template <class T>
class Mixin : public Adapter
{
public:
  //< Replace "Types ... args" with "Types& ... args" and it works even with Apple LLVM
  template <class ... Types>
  Mixin(Types ... args) : Adapter(T(args...)) { }
};

int main()
{
  std::vector<int> data;
  data.push_back(5);
  data.push_back(42);
  Mixin<std::vector<int>> mixin(data);
  std::cout << "data:  " << data.size() << "\n";
  std::cout << "mixin: " << mixin.GetSize() << "\n";
  return 0;
}

Result using Apple LLVM, tested with -std=c++11 and -std=c++14:

data:  2
mixin: -597183193

Interestingly, I've tested this code also @ideone which uses gcc-5.1 with C++14 enabled, and it works as expected!

data:  2
mixin: 2

Why does mixin.GetSize() return a garbage value on Clang and why does it work with GCC-5.1?

@Gene suggested that I'm using Types ... args which creates a temporary copy of the vector (and using Types& ... args makes it work with LLVM), but that copy would contain the same elements (thus also have the same size).

Upvotes: 0

Views: 86

Answers (1)

ildjarn
ildjarn

Reputation: 62975

You have a dangling reference, and mixin.GetSize() is yielding undefined behavior:

  • Inside of Mixin's constructor, T = std::vector<int>, so Adapter(T(args...)) is passing Adapter's constructor a temporary std::vector<int>
  • Adapter's constructor parameter is a Wrapper&&, but we're passing it a std::vector<int>&&, so we invoke Wrapper's implicit conversion constructor
  • Wrapper's constructor parameter is a std::vector<int> const&, and we're passing it a std::vector<int>&&; rvalues are allowed to bind to const-lvalue references, so this is syntactically fine and compiles fine, but in effect we're binding Wrapper::bc to a temporary
  • Once construction is finished, the lifetime of the temporary created in Mixin's constructor ends, and Wrapper::bc becomes a dangling reference; calls to Adapter::GetSize now yield UB

When Mixin's constructor parameters are changed from Types... to Types&..., Adapter(T(args...)) is still passing Adapter's constructor a temporary std::vector<int>; it only appears to work because you are seeing a different manifestation of UB (likely the stack looks a bit different due to one fewer copies of std::vector<int> being made). I.e., both versions of the code are equally broken/wrong!

So, to answer this concretely:

Why does mixin.GetSize() return a garbage value on Clang and why does it work with GCC-5.1?

Because the behavior of undefined behavior is undefined. ;-] Appearing to work is one possible outcome, but the code is still broken and the appearance of being correct is purely superficial.

Upvotes: 3

Related Questions