Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed use of =default for vector types #543

Closed
wants to merge 4 commits into from
Closed

Removed use of =default for vector types #543

wants to merge 4 commits into from

Conversation

patrikhuber
Copy link

Defaulting the copy constructor and copy assignment operators causes compiler errors when the type T is not trivially copyable. In that case, explicitly defined copy ctor and assignment are required. This PR removes the #ifdefs around the existing operators, so they're always used.

Defaulting the copy constructor and copy assignment operators causes compiler errors when the type T is not trivially copyable. In that case, explicitly defined copy ctor and assignment are required. This PR removes the #ifdefs around the existing operators, so they're always used.
@patrikhuber
Copy link
Author

patrikhuber commented Aug 17, 2016

This PR does not deal with the case yet when GLM_FORCE_NO_CTOR_INIT is #defined. If you approve of this PR, I'll update it so it fixes that case too.

@Groovounet
Copy link
Member

I don't think this is the proper fix but that should be or great help to build a define allowing the behavior you are looking for.

Thanks,
Chirstophe

@patrikhuber
Copy link
Author

patrikhuber commented Aug 18, 2016

Are you considering merging this PR? It would be nice to lift this artificial restriction of not being able to put in non-trivially copyable types because of the =default. As it is now, non-trivially-copyable types are allowed when the compiler doesn't have =default, and they're not when the compiler has it. That's a weird inconsistency. And actually there's no reason to prevent putting in non-trivially-copyable types at this level.
Also the PR does not change any behaviour if you put in POD types.

In my experience so far with my custom type as T in vec* / mat* / quat*, all of GLMs functionality I've tried worked without any issues whatsoever, the basic math operations work out of the box as well as functions like glm::project and so on. I think that's a big gain.

Thanks very much,
Patrik

Groovounet added a commit that referenced this pull request Sep 1, 2016
@Groovounet Groovounet self-assigned this Sep 2, 2016
@Groovounet Groovounet added this to the GLM 0.9.8 milestone Sep 2, 2016
@Groovounet
Copy link
Member

This issue is partially fixed in master and 0.9.8 branches. I still need to add tests to be good to go.

Thanks for reporting,
Christophe

@Groovounet Groovounet closed this Sep 2, 2016
@patrikhuber
Copy link
Author

Looks great! Thank you very much for your effort! :-)

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

Successfully merging this pull request may close these issues.

2 participants