safe::impl::MutableIfNotReference<MyType> will not run MyType constructor when compiled under VS2015

[jorgenhs]

Using safe library c978f80, the attached program will output Mutex constructed Value constructed when compiled under VS2017, but only Value constructed when compiled under VS2015.

ConstructionTest.cpp.txt

[jorgenhs]

It seems disambiguating the constructors will fix the issue is VS2015: MutableIfNotReference() = default; -template<typename... Args> -explicit MutableIfNotReference(Args&&... args): -get(std::forward<Args>(args)...) +template<typename Arg0, typename... Args> +explicit MutableIfNotReference(Arg0&& arg0, Args&&... args) : + get(std::forward<Arg0>(arg0), std::forward<Args>(args)...) {}

[LouisCharlesC]

Hey, I'm not sure to understand the issue. In the attached file, the line that should construct a value is commented out. But I guess this is the one that has a different behavior under VS2017 and VS2015 ? If it is so, then truly the correct output is "Mutex constructed Value constructed" and VS2015 is wrong.

I don't understand why VS2015 gets it wrong (do you ?) but I guess I see what your fix does. Looking at the code, though, I realize that the variadic constructor covers the default construction and we simply don't need the default constructor statement.

Can you answer these two questions for me, and we'll see how we go along with this:

1. Do the tests pass on VS2015, when the issue happens ?
2. Since we don't need the default constructor, can you tell me what happens if you remove it (see attached file) ? Does it solve the problem ?

Thanks for taking the time to share this with me, Cheers! L-C

mutableref.h.txt

[jorgenhs]

Hi @LouisCharlesC , thank you for the rapid response.

I am very sorry for the confusion, indeed the wrong lines are commented out in the code example. The main function should be, as you correctly inferred,

int main()
{
  safe::Safe<FTestValueType, FTestMutexType> X;
  return 0;
}

I have tested simply removing the default constructor, and that seems to have the same effect. This is also a much cleaner fix. :)

- MutableIfNotReference() = default;

  template<typename... Args>
  explicit MutableIfNotReference(Args&&... args) :
    get(std::forward<Args>(args)...)
  {}

I have not had the opportunity to run the tests yet.

[LouisCharlesC]

Stupid C++, if you don't default (or delete) the default constructor, then the type becomes non-trivial. Who knows what else can happen if we don't do exactly the right thing. Solution: do as little as possible. Here's another mutableref.h.txt, could you try it out ?

[LouisCharlesC]

Blimey! This file alone isn't going to work.

Just pull my last commit and check out if it fixes your problem. If so, we're all set!

[LouisCharlesC]

Tests on Windows added. Also I confirmed that the tests detected this bug on VS2015. Bug was already fixed by previous commit.