static_assert's in the stl containers break SFINAE

[llvmbot]

Bugzilla Link	23014
Resolution	INVALID
Resolved on	Mar 30, 2015 10:31
Version	3.5
OS	MacOS X
Reporter	LLVM Bugzilla Contributor
CC	@mclow

Extended Description

There's a static_assert in all of the STL containers verifying that: std::allocator::value_type == value_type

They appear to have been added in attempt to fix this bug report: https://llvm.org/bugs/show_bug.cgi?id=15576

This means one cannot attempt to use SFINAE with such invalid types, as in the following example code: template<typename T, typename U, typename = void> struct attempt_rebind { using type = T; };

template<template class Container, typename T, typename U, typename... OtherArgs> struct attempt_rebind< Container<T, OtherArgs...>, U, std::enable_if_t< std::is_constructible< Container<T, OtherArgs...>, Container<U, OtherArgs...>

::value

{ using type = Container<U, OtherArgs...>; };

static_assert( std::is_same< std::vector<int, std::allocator>, typename attempt_rebind<std::vector, double>::type

::value, "This should pass!" );

Compilation of this valid code is blocked by the static_assert in std::vector.

Thank you!

[llvmbot]

I'm closing this as invalid. Please reopen this if you disagree.

[llvmbot]

The table says, Allocators are required to be CopyConstructible, yet InvalidAllocator most definitely is not, and the code still compiles. My real point of confusion, is why InvalidAllocator compiles, even though it fails all (most of) the must have requirements with the exception of the nested typedef. Yet removing the nested typedef, or making that typedef not meet the requirements of an AllocatorAwareContainer blocks compilation.

Allocator::value_type is treated differently because it using Allocator<Base>::allocate(...) in a vector<Derived> will compile and silently do the wrong thing at runtime. For this reason we explicitly check that Allocator::value_type == value_type. The goal of the static assert is to diagnose code like this (that would compile without the static assert).

struct Base { int x; };
struct Derived : public Base { int y; };

int main() {
    std::vector<Derived, std::allocator<Base>> v(100);
}

Other requirements on Allocator end up getting checked at compile time when methods that try to use the allocator in an invalid way get instantiated. For example the following code won't compile because InvalidAllocator is not CopyConstructible.

template <class Tp>
struct InvalidAllocator : std::allocator<Tp> {
    InvalidAllocator(const InvalidAllocator<Tp>&) = delete;
};

void foo() {
   std::vector<int, InvalidAllocator<int>> v1;
}

If the value_type assert is right. Why not have the STL containers have this static_assert that would block the InvalidAllocator example from compiling: static_assert(std::is_default_constructible::value, "Allocator must be default constructible.");

If it doesn't need to be checked with a static assert then I don't think we should check it. Eagerly checking these requirements would be error prone and unneeded. Also I think this might prevent vector from being used with incomplete types.

I definitely don't think you should ever be able to create an instance of such a container, but for metafunctions that don't generate such a container.

Your free to use metafunctions that name vector<int, InvalidAllocator> but as soon as you instantiate that vector your are bound by the requires clauses in the standard.

I hope this helped.

[llvmbot]

I think I'm not explaining this properly, hopefully this response helps. Thanks again for responding.

It shouldn't compile. Allocators are required by the standard to contain a nested typedef value_type.

Take a look at this table to see what an allocator must have and what it can have. http://en.cppreference.com/w/cpp/concept/Allocator

The table says, Allocators are required to be CopyConstructible, yet InvalidAllocator most definitely is not, and the code still compiles. My real point of confusion, is why InvalidAllocator compiles, even though it fails all (most of) the must have requirements with the exception of the nested typedef. Yet removing the nested typedef, or making that typedef not meet the requirements of an AllocatorAwareContainer blocks compilation. It seems like the value_type requirement is treated differently than the other requirements.

std::vector<T, Alloc> uses Alloc to allocate items of type T. If Alloc::allocate(...) does not allocate items of type T then bad things happen inside vector. Simply put the allocator must allocate the same type that is stored in the vector. The standard requires that T == Alloc::value_type in order to use vector. For this reason it makes sense to have that static assert.

I definitely don't think you should ever be able to create an instance of such a container, but for metafunctions that don't generate such a container, I don't understand why value_type blocks compilation, and the other must requirements don't. (Note: InvalidAllocator lacks the required A::pointer allocate(n)).

If the value_type assert is right. Why not have the STL containers have this static_assert that would block the InvalidAllocator example from compiling: static_assert(std::is_default_constructible::value, "Allocator must be default constructible.");

Thanks again.

[llvmbot]

I guess my point was: if you commented out value_type in InvalidAllocator, it should also compile (but it doesn't with clang 3.5 anyway). Is there any reason getting rid of value_type should be any worse than getting rid of the default constructor, or any other attributes that defines an allocator? I may be missing something here, but I'm not sure what yet.

It shouldn't compile. Allocators are required by the standard to contain a nested typedef value_type.

Take a look at this table to see what an allocator must have and what it can have. http://en.cppreference.com/w/cpp/concept/Allocator

Also, if you change: using value_type = Tp; to using value_type = float; // or some other wrong type

It fails to compile, specifically because of the static_assert.

I don't understand why, that static_assert is needed there.

std::vector<T, Alloc> uses Alloc to allocate items of type T. If Alloc::allocate(...) does not allocate items of type T then bad things happen inside vector. Simply put the allocator must allocate the same type that is stored in the vector. The standard requires that T == Alloc::value_type in order to use vector. For this reason it makes sense to have that static assert.

[llvmbot]

Also, if you change: using value_type = Tp; to using value_type = float; // or some other wrong type

It fails to compile, specifically because of the static_assert.

I don't understand why, that static_assert is needed there.

[llvmbot]

I don't understand your point...

Your test case never instantiates Container<Value_type, InvalidAllocator<Value_type>>. It's not SFINAE friendly it is just unevaluated.

Can you expand and explain what you think it should do?

I think that example should do exactly what it does.

I guess my point was: if you commented out value_type in InvalidAllocator, it should also compile (but it doesn't with clang 3.5 anyway). Is there any reason getting rid of value_type should be any worse than getting rid of the default constructor, or any other attributes that defines an allocator? I may be missing something here, but I'm not sure what yet.

Thanks for responding.

[llvmbot]

I don't understand your point...

Your test case never instantiates Container<Value_type, InvalidAllocator<Value_type>>. It's not SFINAE friendly it is just unevaluated.

Can you expand and explain what you think it should do?

[llvmbot]

I'm not great at reading standardese, so maybe it doesn't violate any rules in the standard, if so, sorry about that. However, I will point out that AllocatorAwareContainer's also require their Allocators be DefaultConstructible. But the following code compiles:

template struct InvalidAllocator { typedef Tp value_type; // comment this out and it fails

InvalidAllocator() = delete;
InvalidAllocator(const InvalidAllocator<Tp>&) = delete;
InvalidAllocator(InvalidAllocator<Tp>&&) = delete;
InvalidAllocator& operator=(const InvalidAllocator<Tp>&) = delete;
InvalidAllocator& operator=(InvalidAllocator<Tp>&&) = delete;

};

template<typename T, typename = void> struct try_change_to_invalid_allocator { using type = T; }; template<template class Container, typename Value_type, typename AllocType> struct try_change_to_invalid_allocator< Container<Value_type, AllocType>, std::enable_if_t< std::is_constructible< Container<Value_type, AllocType>, Container<Value_type, InvalidAllocator>

::value

{ using type = Container<Value_type, InvalidAllocator>; };

static_assert( std::is_same< std::vector<int, std::allocator>, typename try_change_to_invalid_allocator<std::vector>::type

::value, "SFINAE friendly" );

// std::vector<int, InvalidAllocator> x; // uncomment this, and it won't compile

So either, libc++, implements the DefaultConstructible requirement in a SFINAE friendly way, or that requirement is implicit based on the way the allocator is used within std::vector

Note the two comments.

[mclow]

I don't see this as a bug.

When the standard says "Requires", that means "You have to satisfy this - otherwise all bets are off"

When you say "use SFINAE with such invalid types", I hear "violating the Requires clause", and so, you have no guarantees of behavior.

Compilation of this valid code is blocked by the static_assert in std::vector. Because of this, I'm not convinced that this is valid code.

[llvmbot]

Sorry, I miswrote what the static_assert verifies. The static_assert verifies: typename allocator_type::value_type == value_type