Fix compilation on Visual Studio 2019 v16.11.42 #8
Conversation
Although Visual Studio 2022 allows function members in a templated class to use `requires` (and this appears to be permitted by the language, although I haven't dug deeper than CPPReference here), Visual Studio 2019 emits Error C7599, "a trailing requires clause is only allowed on a templated function" when attempting to compile members of `linear_memory_resource`. My solution is to make these functions templated as well, with a default template argument equal to the class' template argument. Since templated constructors can't be defaulted, I've added the default implementation there as well. Signed-off-by: Nia Bickford <nbickford@nvidia.com>
WalkthroughThe pull request focuses on modifying the Changes
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
include/decodeless/allocator.hpp(3 hunks)
🔇 Additional comments (5)
include/decodeless/allocator.hpp (5)
91-96: LGTM! Explicit initialization maintains the original behavior.The explicit initialization of
m_parentis correct and necessary since templated constructors cannot be defaulted.
76-78: LGTM! Template workaround is consistently applied.The template workaround is uniformly applied across all constructors, maintaining consistency and readability.
Also applies to: 99-101, 108-110
61-62: LGTM! Template parameter defaulting is safe.The dummy template parameter
U = ResOrAllocis correctly defaulted and won't interfere with the existing template machinery.Also applies to: 77-78, 100-101, 109-110
Line range hint
58-113: LGTM! Changes successfully address MSVC 2019 compilation issues.The modifications effectively work around the MSVC 2019 limitation regarding trailing requires clauses while maintaining the original functionality. The solution is:
- Minimal and focused on the compiler issue
- Consistent across all affected constructors
- Safe with respect to template instantiation and memory management
Line range hint
62-74: Verify that the changes don't affect memory allocation behavior.While the changes appear to be purely syntactic, let's verify that no memory-related behaviors have been altered.
Also applies to: 79-87, 102-104, 111-113
✅ Verification successful
Type constraints don't affect runtime memory behavior
The changes only add compile-time type constraints through the
requires allocator<ResOrAlloc>clause, with no modifications to the actual memory allocation logic or safety checks. The constructor's runtime behavior remains identical to the original implementation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any changes to memory allocation patterns rg -l "allocate|deallocate|reallocate" > allocation_files.txt git diff origin/main...HEAD -- $(cat allocation_files.txt) | grep -E "^[\+\-].*\b(allocate|deallocate|reallocate)\b"Length of output: 350
Script:
#!/bin/bash # Check implementation and usage of allocate_bytes ast-grep --pattern 'allocate_bytes<$_>($_,$_)' # Look for allocation-related concept definitions ast-grep --pattern 'concept $_ = $_<$_> && requires($_) { $$$; };' # Check for any memory resource implementations ast-grep --pattern 'class $_ : public memory_resource { $$$ }'Length of output: 182
| #if _MSC_VER < 1930 | ||
| // MSVC 2019 doesn't allow a trailing requires clause on a function of a | ||
| // templated class. Adding a dummy template like this avoids error C7599. | ||
| template<class U = ResOrAlloc> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider marking this as a temporary workaround.
The compiler version check and template workaround effectively addresses the MSVC 2019 limitation with trailing requires clauses.
Consider adding a TODO comment to remove this workaround once MSVC 2019 support is no longer needed:
#if _MSC_VER < 1930
+ // TODO: Remove this workaround once MSVC 2019 support is dropped
// MSVC 2019 doesn't allow a trailing requires clause on a function of a
// templated class. Adding a dummy template like this avoids error C7599.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #if _MSC_VER < 1930 | |
| // MSVC 2019 doesn't allow a trailing requires clause on a function of a | |
| // templated class. Adding a dummy template like this avoids error C7599. | |
| template<class U = ResOrAlloc> | |
| #if _MSC_VER < 1930 | |
| // TODO: Remove this workaround once MSVC 2019 support is dropped | |
| // MSVC 2019 doesn't allow a trailing requires clause on a function of a | |
| // templated class. Adding a dummy template like this avoids error C7599. | |
| template<class U = ResOrAlloc> |
pknowlesnv
left a comment
pknowlesnv
left a comment
There was a problem hiding this comment.
LGTM. Half tempted to try class U = void just because it avoids confusion over U being used/expected to match, but not that important. TY!! :)
3 participants
Although Visual Studio 2022 allows function members in a templated class to use
requires(and this appears to be permitted by the language, although I haven't dug deeper than CPPReference here), Visual Studio 2019 emits Error C7599, "a trailing requires clause is only allowed on a templated function" when attempting to compile members oflinear_memory_resource.My solution is to make these functions templated as well, with a default template argument equal to the class' template argument. Since templated constructors can't be defaulted, I've added the default implementation there as well.
Thanks!
Summary by CodeRabbit
linear_memory_resourceclass to improve compiler compatibility