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

Update TinySTL (attempt 2). #341

Merged
merged 1 commit into from
Jan 30, 2025
Merged

Conversation

mcourteaux
Copy link
Contributor

@mcourteaux mcourteaux commented Jan 27, 2025

A better-tested updated version of TinySTL. Replaces #340.
This library was in a very buggy state, so I made my own fork of it, patched all the issues, and made a PR there (mendsley/tinystl#27).

I added some extra tests that were present in the tinystl repo, but not yet in the bx testsuite. All tests are passing. This time I did test the examples, and I don't get any crashes.

Also includes a small update-script to pull it the sources from the original repo and replace all #include <TINYSTL/xxx> with #include <tinystl/xxx> using sed.

Idk if there are cool tricks in Github to rewrite git history as if my first PR was never merged (instead of merged, reverted, and then this merged).

@bkaradzic bkaradzic merged commit 73966ef into bkaradzic:master Jan 30, 2025
9 checks passed
bkaradzic added a commit that referenced this pull request Jan 30, 2025
@bkaradzic
Copy link
Owner

@bkaradzic
Copy link
Owner

Instead updating the whole thing, you should fix whatever you need to fix in bx.

@mcourteaux
Copy link
Contributor Author

mcourteaux commented Jan 30, 2025

It's not an error. Was a type-cast warning elevated to error-level. Fixed in #342, checked the CI bots now for warnings. Good to know that's a thing. Real issue here is that bx is compiled with lower warning-to-error-escalation settings than bgfx. Therefore CI test on bx passed, but failed on bgfx.

@bkaradzic
Copy link
Owner

Real issue here is that bx is compiled with lower warning-to-error-escalation settings than bgfx.

It shouldn't... Where is the difference?

@mcourteaux
Copy link
Contributor Author

mcourteaux commented Jan 31, 2025

On the one hand (warning as error in bgfx):
https://github.com/bkaradzic/bgfx/actions/runs/13058550303/job/36435625648#step:6:3202

D:\a\bgfx\bgfx\bx\include\tinystl\string.h(251,12): error C2220: the following warning is treated as an error [D:\a\bgfx\bgfx\bgfx\.build\projects\vs2022\examples.vcxproj]
  			int len = m_last - m_first;
  			        ^ (compiling source file '../../../examples/47-pixelformats/pixelformats.cpp')
  
D:\a\bgfx\bgfx\bx\include\tinystl\string.h(251,12): warning C4244: 'initializing': conversion from '__int64' to 'int', possible loss of data [D:\a\bgfx\bgfx\bgfx\.build\projects\vs2022\examples.vcxproj]

On the other hand (bx):

https://github.com/bkaradzic/bx/actions/runs/12992772828/job/36233405585

Warnings not even printed, for a test string_test.cpp using tinystl::string:
https://github.com/bkaradzic/bx/pull/341/files#diff-c5446f4c1753d974786291fb72e312a4cf3382866566436b403c088f781cb052R642

Perhaps the bx test suite doesn't have the same warnings enabled?

@bkaradzic
Copy link
Owner

Found the issue...

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