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

add -D_GLIBCXX_ASSERTIONS to CXXFLAGS #793

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZeroChaos-
Copy link
Sponsor

Many linux distributions have switched this on by default, but TR does not have it on by default and so bugs are slipping through and hitting users. Turn on bounds checking by default so that bugs are found quicker.

This WILL break users. I'm sure of it. It will cause program exit when running broken code, example being #783 The performance penalty of this is minimal (none on modern distros where this is already teh default) and the bounds checking causing failure is better than simply running objectively wrong code.

Many linux distributions have switched this on by default, but TR does
not have it on by default and so bugs are slipping through and hitting
users.  Turn on bounds checking by default so that bugs are found
quicker.

This WILL break users.  I'm sure of it.  It will cause program exit when
running broken code, example being robotastic#783
The performance penalty of this is minimal (none on modern distros where
this is already teh default) and the bounds checking causing failure is
better than simply running objectively wrong code.
@ZeroChaos-
Copy link
Sponsor Author

#783
#780

@robotastic
Copy link
Owner

This is 🔥! I like it, but I probably have to get a host of people to run this on test installs first before turning it on for everyone. I am scared of all the ways it will break.

How about I create a branch with this on, and we can collect all of the fixes in that branch. Once we are getting good uptime with a number of different system types, we can mainline it?

@ZeroChaos-
Copy link
Sponsor Author

I leave that up to you. I will reiterate now that this will only cause breakage on objectively wrong code. It may be better to find it all quickly and rip off the band aid. Additionally, this is a default use flag for most modern major distros, so time is short here and more and more users will be hitting runtime errors. I support your solution, but I do feel pulling into master will get much wider testing much faster. You may choose to fix the bug I already found first ;-)

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.

None yet

2 participants