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

Fix MSVC Warnings With “emplace_back()” #4889

Merged
merged 2 commits into from Jan 23, 2023

Conversation

krishty
Copy link
Contributor

@krishty krishty commented Jan 17, 2023

Several places in the code call std::vector<aiVector3D>.emplace_back(0, 0, 0). The constructor of aiVector3D actually expects arguments of the type ai_real, (alias of float if compiling without ASSIMP_DOUBLE_PRECISION) but the literal 0 is of type int.

emplace_back() does support promotion, but int to float is a potentially lossy conversion. tl;dr: On warning level 4, MSVC spits out a very deeply nested warning C4244: 'argument': conversion from '_Ty' to 'TReal', possible loss of data with _Ty=int and TReal=ai_real.

I used static_cast<ai_real>() in the affected places because both ai_real(0) and (ai_real)0 are regarded as C-style casts and frowned upon by various linters, e.g. clang-tidy.

Krishty and others added 2 commits January 18, 2023 00:08
Several places in the code call `std::vector<aiVector3D>.emplace_back(0, 0, 0)`. The constructor of `aiVector3D` actually expects arguments of the type `ai_real`, (alias of `float` if compiling without `ASSIMP_DOUBLE_PRECISION`) but the literal `0` is of type `int`.

`emplace_back()` does support promotion, but `int` to `float` is a potentially lossy conversion. tl;dr: On warning level 4, MSVC spits out a very deeply nested `warning C4244: 'argument': conversion from '_Ty' to 'TReal', possible loss of data with _Ty=int and TReal=ai_real`.
Copy link
Member

@kimkulling kimkulling left a comment

Choose a reason for hiding this comment

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

Looks fine.

@kimkulling kimkulling merged commit 8463242 into assimp:master Jan 23, 2023
@kimkulling
Copy link
Member

Merged, thanks a lot for your contribution.

@krishty krishty deleted the fix-msvc-emplace-warnings branch January 24, 2023 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants