Fix modern MSVC compilation error on experimental/filesystem#8469
Conversation
bob80905
changed the title
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| namespace fs = std::filesystem; | ||
| #else | ||
| #include <experimental/filesystem> | ||
| namespace fs = std::experimental::filesystem; |
There was a problem hiding this comment.
Not sure which compiler this else case covers?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 <filesystem>.
| namespace fs = std::filesystem; | ||
| #endif | ||
| #elif defined(__has_include) && __has_include(<filesystem>) && \ | ||
| (defined(__clang__) || !defined(__GNUC__) || __GNUC__ >= 8) |
There was a problem hiding this comment.
why do we need to check && \ (defined(__clang__) || !defined(__GNUC__) || __GNUC__ >= 8) why isn't just checking for the feature sufficient?
There was a problem hiding this comment.
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."
farzonl
left a comment
farzonl
left a comment
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
| #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; |
There was a problem hiding this comment.
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 <filesystem>.
3 participants
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.