Data<T>::Part: SFINAE-guard forwarding ctors against self / derived types (MSVC C4717) - #2
Merged
Merged
Conversation
Same fix as the examples/ sweep -- stop build_unflags from stripping C++20+, keep C++17 as floor not ceiling. .RnD stays local per project convention; committing here only so the fix isn't lost to a future clean checkout, not for push.
…ypes MSVC's overload resolution picks the variadic Part(OO&&...) forwarding ctor for Fir(const Fir&) instead of the implicit copy ctor, then recurses one Chain::Part level per `using Base::Base` and C4717s into a runtime stack overflow. GCC and Clang pick the copy ctor and never hit it, so this only shows on the MSVC CI leg (OneHLS test/staticList_test.cpp, which copy-constructs a Fir<> into a StaticList). Guard both forwarding ctors so a single argument that is (a copy/move of) Part -- or a type derived from it -- cannot select them; the implicit copy/move ctor wins. Value-and-base-args and default construction are unaffected. Verified: OneData/OneHLS test suites (g++, clang++, C++17/20/23), OneHLS staticList_test.cpp, downstream OneItem/OneOutput/OneMenu tests (identical pass with and without the change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…late default-arg) The first attempt used a C++17 fold expression inside the enable_if_t default-argument; MSVC rejects that with C2059. Replace with a nested one-arg-specialized trait, no fold. GCC/Clang/C++17/20 unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
MSVC's overload resolution picks the variadic
Part(OO&&...)forwardingconstructor for
Fir(const Fir&)instead of the implicit copy ctor, thenrecurses one
Chain::Partlevel perusing Base::Baseand C4717s intoa runtime stack overflow. GCC and Clang pick the copy ctor and never
hit it — this only shows on the recently-added MSVC CI leg (via
OneHLS test/staticList_test.cpp, which copy-constructs aFir<>into aStaticList).Fix
SFINAE-guard both
Data<T>::Partforwarding ctors so a single argumentthat is (a copy/move of)
Part— or a type derived from it, e.g.Fir<>— cannot select them; the implicit copy/move ctor wins. The
value-plus-base-args path (
Part(v, base_args...)) and defaultconstruction are unaffected.
Verified (local, g++ 13 / clang++ 18)
test/test.cpp— g++, clang++,-std=c++17/20/23test/test.cpp(ac_fixed + ap_fixed) andtest/staticList_test.cpp(the case that stack-overflows on MSVC) — pass
with and without the change
examples/hls_streaming_buffers(16 tests + demo) — passMSVC itself can't be tested locally; CI will confirm. This is the
textbook remedy for a greedy forwarding ctor shadowing the copy ctor.
🤖 Generated with Claude Code