Skip to content

Fix modern MSVC compilation error on experimental/filesystem #8469

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.

Sign up for GitHub

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

Open
bob80905 wants to merge 6 commits into
base: main
Choose a base branch
from
+19 −7
Open

Fix modern MSVC compilation error on experimental/filesystem #8469

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cbdebaf
fix modern MSVC
bob80905 May 21, 2026
521ae42
repair
bob80905 May 21, 2026
b433d21
format
bob80905 May 21, 2026
3aaa95d
fix missing endif
bob80905 May 21, 2026
54e02ab
fix macos
bob80905 May 22, 2026
6b84fe1
chore: autopublish 2026-05-22T17:58:08Z
github-actions[bot] May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions tools/clang/unittests/HLSL/DxilContainerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,27 @@
#include <atlfile.h>
#include <d3dcompiler.h>
#pragma comment(lib, "d3dcompiler.lib")
#if _MSC_VER >= 1920
#define _SILENCE_EXPERIMENTAL_FILESYSTEM_DEPRECATION_WARNING
#include <experimental/filesystem>
#else
#include <filesystem>
#endif

#if defined(_MSC_VER)
// MSVC removed <experimental/filesystem> starting in VS 2019 16.3 (_MSC_VER >= 1922)
#if _MSC_VER < 1922
Copy link
Copy Markdown
Member

@hekota hekota May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#if _MSC_VER < 1922
#if (_MSC_VER >= 1920) && (_MSC_VER < 1922)

experimental::filesystem was enabled since version 1920, so it needs to be included in the check.

#include <experimental/filesystem>
namespace fs = std::experimental::filesystem;
#else
#include <filesystem>
namespace fs = std::filesystem;
#endif
#elif defined(__has_include) && __has_include(<filesystem>) && \
(defined(__clang__) || !defined(__GNUC__) || __GNUC__ >= 8)
Copy link
Copy Markdown
Collaborator

@farzonl farzonl May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to check && \ (defined(__clang__) || !defined(__GNUC__) || __GNUC__ >= 8) why isn't just checking for the feature sufficient?

Copy link
Copy Markdown
Collaborator Author

@bob80905 bob80905 May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for GNUC >= 8: GCC 7 ships but doesn't actually define std::filesystem. Header exists, namespace doesn't -> compile error. GCC 8 is the first version where it really works.
For the defined(clang) check: Clang sets GNUC == 4, so the >= 8 check would wrongly reject it. The clause basically says "if it's Clang, skip the GCC version check."

#include <filesystem>
namespace fs = std::filesystem;
#else
#include <experimental/filesystem>
namespace fs = std::experimental::filesystem;
Copy link
Copy Markdown
Collaborator

@farzonl farzonl May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure which compiler this else case covers?

Copy link
Copy Markdown
Collaborator Author

@bob80905 bob80905 May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consulted an AI, and basically it looks like really old compilers would be covered by this case:
GCC 5.3 – 7.x
Pre-2014 GCC / Clang without __has_include
Clang with an old libstdc++/libc++

are some examples.

Copy link
Copy Markdown
Collaborator

@farzonl farzonl May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even support compilers that old for building dxc? LLVM has a cut off. Not sure if DXC ever required a minimum compiler version.

Copy link
Copy Markdown
Member

@hekota hekota May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCC 8 came out in 2018, and I don't think we should support compilers older than that. I think you can remove the #elif`` condition and just fall back to #include `.

#endif

using namespace fs;

#include "llvm/Support/Format.h"
#include "llvm/Support/raw_ostream.h"
Expand Down Expand Up @@ -72,8 +86,6 @@ using namespace std;
using namespace hlsl_test;
#ifdef _WIN32

using namespace std::experimental::filesystem;

static uint8_t MaskCount(uint8_t V) {
DXASSERT_NOMSG(0 <= V && V <= 0xF);
static const uint8_t Count[16] = {0, 1, 1, 2, 1, 2, 2, 3,
Expand Down