fix(craft): delete copy ops on classes holding unique_ptr maps - #93
Merged
Merged
Conversation
MSVC's __declspec(dllexport) (DOCRAFT_LIB, active when DOCRAFT_BUILD_SHARED_LIBS is set) forces instantiation of every public member of an exported class, including implicitly-defined copy constructor/assignment. std::unordered_map<K, std::unique_ptr<V>>'s own copy assignment isn't SFINAE-disabled based on value_type (a pre-C++20 container quirk), so the implicit copy ops on DocraftLoomTagHandlerRegistry and DocraftCraftLanguageParser were never deleted at the type level -- only broken when actually instantiated. GCC/Clang's static-lib builds never instantiate unused implicit members, so this only surfaces on a Windows/MSVC shared-lib build (error C2679 trying to copy-assign the unique_ptr-valued map). Neither class is ever copied (registry is a singleton; the parser is always used as a local) -- explicitly deleting copy ctor/assignment on both fixes the DLL export instantiation without changing any call site. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015RjTSLobQWMZq8DcFyDZoG
|
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.
MSVC's __declspec(dllexport) (DOCRAFT_LIB, active when DOCRAFT_BUILD_SHARED_LIBS is set) forces instantiation of every public member of an exported class, including implicitly-defined copy constructor/assignment. std::unordered_map<K, std::unique_ptr>'s own copy assignment isn't SFINAE-disabled based on value_type (a pre-C++20 container quirk), so the implicit copy ops on DocraftLoomTagHandlerRegistry and DocraftCraftLanguageParser were never deleted at the type level -- only broken when actually instantiated. GCC/Clang's static-lib builds never instantiate unused implicit members, so this only surfaces on a Windows/MSVC shared-lib build (error C2679 trying to copy-assign the unique_ptr-valued map).
Neither class is ever copied (registry is a singleton; the parser is always used as a local) -- explicitly deleting copy ctor/assignment on both fixes the DLL export instantiation without changing any call site.
Claude-Session: https://claude.ai/code/session_015RjTSLobQWMZq8DcFyDZoG