2

I just found a bug in my code and I am pretty baffled that it could happen, since it is a simple signed/unsigned mismatch - which should not happen at all, because I am compiling with warning level 4, warnings as errors. So I tried to reproduce it, which is pretty simple:

#include <memory>

class MyClass {
public:
    MyClass( unsigned ) {}
};
int main()
{
    MyClass* rawP = new MyClass(-1);                // issues a warning, as expected
    auto uniqueP = std::make_unique<MyClass>(-1);   // NO WARNING??!

    // silence the compiler
    rawP; 
    uniqueP;

    return 0;
}

Now I'm asking myself: What's the reason of this? Is it a bug in VS, or is it a general shortcoming of std::make_unique? Is there any way to fix it? (Visual Studio 2015 Community Update 3)

user2328447
  • 1,807
  • 1
  • 21
  • 27

1 Answers1

1

You are seeing a combination of several effects.

  1. The call in your main() is perfectly legitimate because the make_unique template instantiation matches the signed data type you feed it.
  2. The implementation of make_unique does not generate a warning because warnings are typically disabled inside system headers.
  3. Visual Studio seems to be unable to detect the potential (but not definite) sign conversion issue inside make_unique.

In more detail:

1. The template instantiation is actually legitimate.

A typical implementation of std::make_unique looks like this (compare cppreference):

template <typename T, typename... Args>
inline std::unique_ptr<T> make_unique(Args&&... args)
{
  return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
}

In your case, as you call std::make_unique<MyClass>(-1), the template is instantiated for a signed integer. Hence, you do not get a warning in your code because no unsigned/signed conversion happens.

2. System headers typically disable warnings.

However, you could rightfully expect a warning from the make_unique implementation. After all, when new T(...) is called with your signed parameter, a signed/unsigned conversion still happens. As a case in point, take the following program:

#include <memory>

class MyClass
{
public:
  MyClass(unsigned) { }
};

template <typename T, typename... Args>
inline std::unique_ptr<T> custom_make_unique(Args&&... args)
{
  return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
}

int main()
{
  auto uniqueP = custom_make_unique<MyClass>(-1);
  (void) uniqueP;
  return 0;
}

When I compile this using GCC with -Wsign-conversion, I get the warning

test.cpp: In instantiation of 'std::unique_ptr<_Tp> custom_make_unique(Args&& ...) [with T = MyClass; Args = {int}]':
test.cpp:17:48: required from here
test.cpp:12:63: warning: conversion to 'unsigned int' from 'int' may change the sign of the result [-Wsign-conversion]
return std::unique_ptr(new T(std::forward(args)...));

So the question is, why do you not get this warning for the std::make_unique() implementation? The answer is essentially that the compiler silences these warnings for its system headers. For example, the GCC version of the <memory> header contains the pragma

#pragma GCC system_header

As soon as this pragma is present in a header file, the compiler no longer reports the warnings for everything inside that header. From the GCC documentation:

The header files declaring interfaces to the operating system and runtime libraries often cannot be written in strictly conforming C. Therefore, GCC gives code found in system headers special treatment. All warnings, other than those generated by ‘#warning’ (see Diagnostics), are suppressed while GCC is processing a system header.

See also this SO post for more details. Presumably a similar approach is taken by Visual Studio's compiler (and as you wrote in your comment, the header temporarily reduces the warning level).

3. Looks like you are encountering a VisualStudio limitation.

In the case of VisualStudio, there is also something else at work. Note how the GCC warning above says that there may be a sign conversion problem (depending on what values users will later feed into custom_make_unique). It appears that VisualStudio can only warn if there is a definite sign conversion issue. See the following program:

#include <iostream>

void f(unsigned) { }

template <typename T>
void g(T val) { f(val); } // GCC issues a warning, VS does NOT

int main()
{
  f(-1); // GCC and VS issue a warning
  g(-1); // no conversion warning here (g<int> instantiated)
}

Try it online.

mindriot
  • 5,413
  • 1
  • 25
  • 34
  • Wow, that's a comprehensive answer, thank you very much. I tried it with your `custom_make_unique()`, but it still doesn't issue a warning (even when also copy and paste a `custom_forward` and `custom_remove_reference` into my own code). At least I now have an idea what to look for. – user2328447 Feb 21 '18 at 20:46
  • Hm. It's possible that VisualStudio, in general, only manages to warn in cases where it can deduce that there _will_ be a sign conversion problem. In my GCC example above, the compiler is only able to deduce that there _may_ be a problem (depending on the values you later feed into `custom_make_unique`). So maybe you are running into a Visual Studio limitation as well. – mindriot Feb 21 '18 at 20:59
  • Indeed, the `memory` header temporarily reduces the warning level to 3 with a `#pragma warning(push,3)`, which would explain the missing warning. Nevertheless, it does not explain why the `custom_make_unique()` also does not work. – user2328447 Feb 21 '18 at 20:59
  • Thanks for checking. I updated my answer (luckily rextester.com provides a VC++ compiler). – mindriot Feb 21 '18 at 21:28