Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSVC 19.x is not working #276

Open
sunavlis opened this issue Jun 12, 2023 · 8 comments
Open

MSVC 19.x is not working #276

sunavlis opened this issue Jun 12, 2023 · 8 comments

Comments

@sunavlis
Copy link

The mdspan library doesn't compile when using it in Visual Studio 2022 and MSVC 19.35.

With the following cMake and example source results in a compilation error:

CMakeLists.txt.

cmake_minimum_required(VERSION 3.13 FATAL_ERROR)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED Yes)

project(mdspanTest
	VERSION 0.0.1
)

set(MDSPAN_CXX_STANDARD ${CMAKE_CXX_STANDARD} CACHE INTERNAL "Use CMAKE_CXX_STANDARD .")
include(FetchContent)
FetchContent_Declare(mdspan
	GIT_REPOSITORY https://github.com/kokkos/mdspan.git
	GIT_TAG mdspan-0.6.0
)
FetchContent_GetProperties(mdspan)
if(NOT mdspan_POPULATED)
	FetchContent_Populate(mdspan)
	add_subdirectory(${mdspan_SOURCE_DIR} ${mdspan_BINARY_DIR} EXCLUDE_FROM_ALL)
endif()

add_executable(${PROJECT_NAME}
  main.cpp
)

target_link_libraries(${PROJECT_NAME} 
	std::mdspan
)

main.cpp

#include <cstddef>
#include <iostream>
#include <experimental/mdspan>
#include <vector>

int main()
{
    std::vector v{ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 };
    auto ms2 = std::experimental::mdspan(v.data(), 2, 6);
}

Error

  [1/2] Building CXX object CMakeFiles\mdspanTest.dir\main.cpp.obj
  FAILED: CMakeFiles/mdspanTest.dir/main.cpp.obj 
  C:\PROGRA~1\MIB055~1\2022\COMMUN~1\VC\Tools\MSVC\1435~1.322\bin\Hostx64\x64\cl.exe  /nologo /TP  -IC:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include /DWIN32 /D_WINDOWS /W3 /GR /EHsc /MDd /Zi /Ob0 /Od /RTC1 -std:c++17 /showIncludes /FoCMakeFiles\mdspanTest.dir\main.cpp.obj /FdCMakeFiles\mdspanTest.dir\ /FS -c C:\Projects\Mdspan_Test\main.cpp
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_stride.hpp(375): warning C4346: 'Extents::rank': dependent name is not a type
  C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_stride.hpp(375): note: prefix with 'typename' to indicate a type
  C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_stride.hpp(475): note: see reference to class template instantiation 'std::experimental::layout_stride::mapping<Extents>' being compiled
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_stride.hpp(375): error C2059: syntax error: '...'
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_stride.hpp(382): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_stride.hpp(382): error C2062: type 'unknown-type' unexpected
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_stride.hpp(384): error C2334: unexpected token(s) preceding '{'; skipping apparent function body
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_stride.hpp(395): error C2270: 'is_exhaustive': modifiers not allowed on nonmember functions
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_stride.hpp(406): error C2270: 'stride': modifiers not allowed on nonmember functions
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(159): warning C4346: 'std::experimental::layout_right::mapping<Extents>::std::experimental::layout_right::mapping<Extents>::extents_type::rank': dependent name is not a type
  C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(159): note: prefix with 'typename' to indicate a type
  C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(220): note: see reference to class template instantiation 'std::experimental::layout_right::mapping<Extents>' being compiled
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(159): error C2059: syntax error: '...'
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(168): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(168): error C2062: type 'unknown-type' unexpected
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(170): error C2334: unexpected token(s) preceding '{'; skipping apparent function body
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(177): error C2270: 'is_unique': modifiers not allowed on nonmember functions
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(178): error C2270: 'is_exhaustive': modifiers not allowed on nonmember functions
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(179): error C2270: 'is_strided': modifiers not allowed on nonmember functions
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\layout_right.hpp(186): error C2270: 'stride': modifiers not allowed on nonmember functions
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\mdspan.hpp(110): error C2059: syntax error: '...'
  C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits/mdspan.hpp(372): note: see reference to class template instantiation 'std::experimental::mdspan<ElementType,Extents,LayoutPolicy,AccessorPolicy>' being compiled
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\mdspan.hpp(119): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\mdspan.hpp(119): error C2062: type 'unknown-type' unexpected
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\mdspan.hpp(123): error C2334: unexpected token(s) preceding ':'; skipping apparent function body
C:\Projects\Mdspan_Test\out\build\x64-Debug\_deps\mdspan-src\include\experimental\__p0009_bits\mdspan.hpp(26): fatal error C1075: '{': no matching token found
  ninja: build stopped: subcommand failed.

Or here in the online demo: https://godbolt.org/z/W9bhGxzqf

Any idea where the error is from? And how it can be fixed?

Thanks,
Silvan

@sunavlis
Copy link
Author

I just found a hint that msvc may contains a compiler bug for fold expression AND: https://stackoverflow.com/questions/56978499/compiler-error-with-a-fold-expression-in-enable-if-t

@mhoemmen
Copy link
Contributor

@sunavlis Thanks for reporting! : - ) This example works on the latest MSVC on Compiler Explorer: https://godbolt.org/z/vxT3vKr6x , when I use /std:c++latest or /std:c++20, but it gives a build error like the one you showed with /std:c++17.

Are you able to build with C++20 or later, or do you actually need C++17 to work for now?

@sunavlis
Copy link
Author

@mhoemmen thanks for your answer. Unfortunately it is true, that our project uses C++17. And at the moment it's not planed to migrate to a newer version of C++.

I did a few changes in the sources and used std::conjunction instead of the _MDSPAN_FOLD_AND. (sunavlis@46b5b25) but now I'm stuck when trying to fix the extents method at: https://github.com/sunavlis/mdspan/blob/stable/include/experimental/__p0009_bits/extents.hpp#L494

@mhoemmen
Copy link
Contributor

@sunavlis Thanks for working on a fix!

but now I'm stuck when trying to fix the extents method....

Is the issue with the fold expression inside MDSPAN_CONDITIONAL_EXPLICIT? The MDSPAN_CONDITIONAL_EXPLICIT macro should be empty in C++17 mode. Nevertheless, could you try just deleting lines 506-510? If that works, we can protect those lines with #if defined(_MSC_VER) && defined(_MSVC_LANG) (as MSVC defines __cplusplus to 199711L unless /Zc:__cplusplus is set, no matter the language version passed to /std).

@sunavlis
Copy link
Author

No the issue isn't in MDSPAN_CONDITIONAL_EXPLICIT. This macro is empty as expected.

The issue is in the __check_compatible_extents function or its parameters used by the macro MDSPAN_TEMPLATE_REQUIRES.

As soon as std::integer_sequence<size_t, Extents...>{} is used, I got the following errors:

C:\Projects\git\github\sunavlis\mdspan\include\experimental\__p0009_bits\extents.hpp(510): error C3546: '...': there are no parameter packs available to expand
  C:\Projects\git\github\sunavlis\mdspan\tests\test_extents.cpp(335): note: see reference to class template instantiation 'Kokkos::extents<size_t,18446744073709551615,18446744073709551615>' being compiled
C:\Projects\git\github\sunavlis\mdspan\include\experimental\__p0009_bits\extents.hpp(512): error C3520: 'Extents': parameter pack must be expanded in this context
  C:\Projects\git\github\sunavlis\mdspan\include\experimental\__p0009_bits\extents.hpp(51): note: see reference to function template instantiation 'std::integral_constant<bool,> Kokkos::detail::__check_compatible_extents(std::integral_constant<bool,true>,std::integer_sequence<size_t,_Vals...>,std::integer_sequence<size_t,OtherExtents...>) noexcept' being compiled

For testing, I changed the __check_compatible_extents implementation and a version without std::integer_sequence<size_t, Extents...>{} compiles. I'm not sure whats happen here...

@mhoemmen
Copy link
Contributor

Thanks for working on this! : - ) I've seen some suggestions to include extra parentheses around (and perhaps also inside) fold expressions.

This week is the C++ Standard Committee plenary meeting, but we should have some more time to look at this in a few days.

@sunavlis
Copy link
Author

Thanks for your reply. In the meantime I found additional interesting facts:

I focused on the differences in MSVC when building with C++17 (/std:c++17) and C++20 (/std:c++20). It seams, that the default value of a compiler flag has changed between this two versions: https://learn.microsoft.com/en-us/cpp/build/reference/permissive-standards-conformance?view=msvc-170

In C++20 the flag is set to /permissive- which enforce the standards conformance where in C++17 it is not set. When the flag is set, the existing code on the stable branch builds also with the current version of MSVC.

Changes are here:
sunavlis@decc8f6

I think it would be nice to have source code which works with many compilers and flags as possible, but with this pragmatic change the library could be used with MSVC with really small changes in the library it self.

What do you think? I could create a merge request to push the changes back into the stable branch.

@crtrott
Copy link
Member

crtrott commented Jun 22, 2023

Yeah we could add that flag, but I also can check if I find another workaround. I am coming back from two weeks of travel including ISO C++ committee today, and might be able to look a bit more into this. That said I am generally all for "make your compiler behave in a standard compliant way" lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants