Skip to content

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