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

Fixed some msvc warnings #548

Merged
merged 3 commits into from
Sep 18, 2016
Merged

Fixed some msvc warnings #548

merged 3 commits into from
Sep 18, 2016

Conversation

amc522
Copy link

@amc522 amc522 commented Sep 12, 2016

Tested on msvc 2015 update 3.

@Groovounet
Copy link
Member

Isn't there any other way to fix this warnings?

@amc522
Copy link
Author

amc522 commented Sep 12, 2016

Not that I could think of. The problem is that the warnings are warning you about the exact thing you are trying to do. I tried a couple of solutions, but nothing worked.

Possibly for the alignment one, something like this could work:
template<std::size_t N> struct aligned {uint8 padding[N]};

Really couldnt figure out anything for the truncation warning though.

@amc522
Copy link
Author

amc522 commented Sep 13, 2016

Did a little better with the struct alignment warnings. The solution is not as elegant as your original one, but it gets the job done. Let me know if the struct alignment replacement is ok.

I tested my changes with the following code:
https://gist.github.com/amc522/fd56b9729fb62a95b6cb5fc8fa52ac5c

Groovounet added a commit that referenced this pull request Sep 14, 2016
Groovounet added a commit that referenced this pull request Sep 14, 2016
@amc522
Copy link
Author

amc522 commented Sep 16, 2016

@Groovounet, just saw the work you did the in 0.9.8-align branch. Thanks for taking the time to cleanup the changes and create the tests! If you need any help from me, let me know.

@Groovounet Groovounet merged commit e57615b into g-truc:master Sep 18, 2016
Groovounet added a commit that referenced this pull request Sep 18, 2016
Groovounet added a commit that referenced this pull request Sep 18, 2016
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