Skip to content

fixed #6316 (Use std::to_string() in place of MathLib::toString() ...) - deleted default implementation of Mathlib::toString()#5341

Merged
chrchr-github merged 2 commits intocppcheck-opensource:mainfrom
firewave:mathlib-str-x
Aug 17, 2023
Merged

fixed #6316 (Use std::to_string() in place of MathLib::toString() ...) - deleted default implementation of Mathlib::toString()#5341
chrchr-github merged 2 commits intocppcheck-opensource:mainfrom
firewave:mathlib-str-x

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

It was also used inconsistently and seemed to imply there is some special handling which wasn't the case. It was just an alias for std::to_string() for non-double types. So there was no need for it.

@firewave
Copy link
Copy Markdown
Collaborator Author

I started these changes separately before I spotted Roberts ticket by accident. Since the changes were just a superset of what he did I credited him as well.

Comment thread test/testmathlib.cpp
ASSERT_EQUALS("0.0", MathLib::toString(0.0));
ASSERT_EQUALS("0.0", MathLib::toString(+0.0));
ASSERT_EQUALS("0.0", MathLib::toString(-0.0));
// float (trailing f or F)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Since we did not have a specialization for float and long double these were actually using the default implementation which is probably not what we wanted. Since we do not use those types in the code these tests could be removed and no further specializations were necessary to implement.

firewave and others added 2 commits August 17, 2023 15:13
…ts for non-existing specializations

Co-authored-by: Robert Reif <reif@earthlink.net>
Co-authored-by: Robert Reif <reif@earthlink.net>
@chrchr-github chrchr-github merged commit 3cf9100 into cppcheck-opensource:main Aug 17, 2023
@firewave firewave deleted the mathlib-str-x branch August 17, 2023 15:52
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