MSVC parameter alignment and ImplicitlyConvertible<From,To> (patch attached)

[Emily Björk]

Visual Studio 2013 has shoddy support for aligning data types. A particular caveat is that formal parameters will not be aligned to the user's requirements.

This means that attempting to pass an object with alignment larger than the default alignment for the type will fail with a compile time error: 

    error C2719: 'xxx': formal parameter with __declspec(align('16')) won't be aligned

This is a problem when using the Eigen linear algebra library. For performance reasons, matrix and vector types are aligned on a 16byte boundary to enable SSE aligned loads. This causes a problem with the ImplicitlyConvertible template here:

gtest-internal.h (line 805)

    static char Helper(To);

    static char (&Helper(...))[2];  // NOLINT

Even though the Helper(To) function is never called, (doesn't even have a body) it causes a C2719 compile error if attempting to use this template for any class which has an alignment requirement larger than the default. 

Although I believe it to be possible to fix this without resorting to C++11 features, this is how we fixed it internally as we already are using C++11. 

    #include <type_traits>

    template <typename From, typename To>

    using ImplicitlyConvertible = std::is_convertible < From, To > ;

and simply removed the original template.

Attached the patch for clarity, it is against our copy of gtest in the repo which includes gmock. The paths may need stripping but otherwise it should work.

[Billy Donahue]

Thanks for the patch, but I don't think std::is_convertible and ImplicitlyConvertible do the same thing.

The "implicitly" is obviously missing from it :).

--

---
You received this message because you are subscribed to the Google Groups "Google C++ Testing Framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email to googletestframe...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Emily Björk]

I believe differently:

http://en.cppreference.com/w/cpp/types/is_convertible

If an imaginary rvalue of type From can be used in the return statement of a function returning To, that is, if it can be converted to To using implicit conversion, provides the member constant value equal to true. Otherwise value is false.

[Billy Donahue]

Corrected!  Thanks.

--

[Emily Björk]

I have found two more cases where alignment causes issues during 32bit compilation on VS 2013.

I have constructed minimal test cases that can reproduce the issues in VS 2013, please fill in the macro for alignment on other compilers. TEST_ALIGNMENT should be set larger than the default alignment for the platform, 32 should do it for most platforms. For reference: Default alignment on 32bit VS2013 is 8, and on 64bit it is 16. 

Regarding HeapAlignedFixtureTest below, the standard only guarantees that new (gtest-internal.h line 484: virtual Test* CreateTest() { return new TestClass; }) will return an address suitably aligned for the largest basic type on the platform. So this is not MSVC specific unfortunately, a custom allocator could fix this but it's an invasive change that would need to be used through out gtest. We work around this using overloaded new/delete in class scope that do aligned allocation. As for AlignedParamTest below we are using a dummy param that has a pointer to a heap allocated, aligned param object, it's is not ideal but it works. 

#if !GTEST_LANG_CXX11

#if _MSC_VER >= 1400

#define alignas(X) __declspec(align(X))

#else

#error "PLEASE_IMPLEMENT alignas for other compilers!"

#endif

#endif

#define TEST_ALIGNMENT 32

struct alignas(TEST_ALIGNMENT) AlignedParam {

    float data[4];

};

class HeapAlignedFixtureTest : public ::testing::Test {

public:

    AlignedParam param;

};

// Causes warning C4316: Object allocated on the heap may not be aligned 32

// This warning should be treated as an error.

TEST_F(HeapAlignedFixtureTest, AddressTest){

    auto this_addr = reinterpret_cast<uintptr_t>(&param);

    ASSERT_EQ(0, this_addr % TEST_ALIGNMENT);

}

class AlignedParamTest : public ::testing::TestWithParam<AlignedParam> {

};

TEST_P(AlignedParamTest, AddressTest){

    auto param_addr = reinterpret_cast<uintptr_t>(&GetParam());

    ASSERT_EQ(0, param_addr % TEST_ALIGNMENT);

}

INSTANTIATE_TEST_CASE_P(XX, AlignedParamTest, ::testing::Values(AlignedParam()));

Compiler output: 

1>d:\work\include\gtest/internal/gtest-param-util.h(433): error C2719: 'parameter': formal parameter with __declspec(align('32')) won't be aligned

1>          d:\work\include\gtest/internal/gtest-param-util.h(446) : see reference to class template instantiation 'testing::internal::TestMetaFactoryBase<AlignedParam>' being compiled

1>          test/test_string.cpp(19) : see reference to class template instantiation 'testing::internal::TestMetaFactory<AlignedParamTest_AddressTest_Test>' being compiled

1>d:\work\include\gtest/internal/gtest-param-util.h(452): error C2719: 'parameter': formal parameter with __declspec(align('32')) won't be aligned

1>d:\work\include\gtest/internal/gtest-param-util-generated.h(79): error C2719: 'v1': formal parameter with __declspec(align('32')) won't be aligned

1>          test/test_string.cpp(24) : see reference to class template instantiation 'testing::internal::ValueArray1<AlignedParam>' being compiled

1>d:\work\include\gtest/internal/gtest-internal.h(484): warning C4316: 'HeapAlignedFixtureTest_AddressxTest_Test' : object allocated on the heap may not be aligned 32

1>          d:\work\include\gtest/internal/gtest-internal.h(484) : while compiling class template member function 'testing::Test *testing::internal::TestFactoryImpl<HeapAlignedFixtureTest_AddressxTest_Test>::CreateTest(void)'

1>          test/test_string.cpp(31) : see reference to class template instantiation 'testing::internal::TestFactoryImpl<HeapAlignedFixtureTest_AddressxTest_Test>' being compiled