Skip to content

Fix modern MSVC compilation error on experimental/filesystem#8469

Open
bob80905 wants to merge 6 commits into
microsoft:mainfrom
bob80905:update_containertest_headers
Open

Fix modern MSVC compilation error on experimental/filesystem#8469
bob80905 wants to merge 6 commits into
microsoft:mainfrom
bob80905:update_containertest_headers

Conversation

@bob80905
Copy link
Copy Markdown
Collaborator

Modern MSVC deperecated experimental/filesystem.
This causes build failures when building DXC with MSVC.
The deprecation occurred on version 1922, so we should account for this removal, instead of silencing deprecation warnings.

@bob80905 bob80905 changed the title Fix modern MSVC compilation error Fix modern MSVC compilation error on experimental/filesystem May 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 14696d1cc10dd5fd802a28a751733747b11022ad 54e02abbd060c9f21a2ef13c6e12317b26b3847b -- tools/clang/unittests/HLSL/DxilContainerTest.cpp
View the diff from clang-format here.
diff --git a/tools/clang/unittests/HLSL/DxilContainerTest.cpp b/tools/clang/unittests/HLSL/DxilContainerTest.cpp
index 98153b24..90c09c66 100644
--- a/tools/clang/unittests/HLSL/DxilContainerTest.cpp
+++ b/tools/clang/unittests/HLSL/DxilContainerTest.cpp
@@ -80,17 +80,17 @@ using namespace fs;
 
 #include <codecvt>
 
-    // clang-format on
+// clang-format on
 
-    using namespace std;
-    using namespace hlsl_test;
+using namespace std;
+using namespace hlsl_test;
 #ifdef _WIN32
 
-    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,
-                                        1, 2, 2, 3, 2, 3, 3, 4};
-      return Count[V];
+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,
+                                    1, 2, 2, 3, 2, 3, 3, 4};
+  return Count[V];
 }
 #endif
 
  • Check this box to apply formatting changes to this branch.

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

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

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

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

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 `.

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

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

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."

Copy link
Copy Markdown
Collaborator

@farzonl farzonl left a comment

Choose a reason for hiding this comment

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

This looks correct. some stuff seems a bit extra but not a reason to hold up the pr.


#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

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.

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

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 `.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

3 participants