Skip to content

Add explicit implementations of TypedAttribute copy/move constructors - #2601

Open
cary-ilm wants to merge 2 commits into
AcademySoftwareFoundation:mainfrom
cary-ilm:typedattribute-constuctor
Open

Add explicit implementations of TypedAttribute copy/move constructors#2601
cary-ilm wants to merge 2 commits into
AcademySoftwareFoundation:mainfrom
cary-ilm:typedattribute-constuctor

Conversation

@cary-ilm

Copy link
Copy Markdown
Member

TypedAttribute's copy/move constructor and assignment operator were declared = default. Under Clang on Windows (msys2 CLANG64), an explicit class-template instantiation (template class TypedAttribute<int> used to export the class from the DLL) does not emit out-of-line definitions for implicitly-defined special members, which leads to undefined symbols at link time.

One more in a series of steps to get OpenEXR building properly on Windows clang.

Assisted-by: GitHub Copilot CLI (model: Claude Sonnet 5)

TypedAttribute<T>'s copy/move constructor and assignment operator were
declared `= default`. Under Clang on Windows (msys2 CLANG64), an
explicit class-template instantiation (`template class
TypedAttribute<int>;`, used to export the class from the DLL) does not
emit out-of-line definitions for implicitly-defined special
members, which leads to undefined symbols at link time.

Assisted-by: GitHub Copilot CLI (model: Claude Sonnet 5)

Signed-off-by: Cary Phillips <seabeepea@gmail.com>

@meshula meshula left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we add a comment to help future us understand this is pattern is deliberate (without having to diff through history)? Something like -

Declare copy constructor, move, and assignment operators explicitly in order to force Clang on Windows to emit these members.

Signed-off-by: Cary Phillips <seabeepea@gmail.com>

@meshula meshula left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants