[kate] [Bug 408539] [KTextEditor] highlighting crash applying invalid format
https://bugs.kde.org/show_bug.cgi?id=408539 --- Comment #6 from Christoph Cullmann --- The assert is there that I/we get a nicer kind of failure in a debug build if one ever breaks that invariant during development and to document the intend. Why should I remove that? If I remove it and break stuff during development, I will need to read a backtrace and figure out the failure. With the assert I get a verbose error message and have the knowledge that it is not wanted to ever arrive with an invalid iterator here. And for the release builds, it produces zero code, the user will not be burdened with unneeded run-time checks. Actually, below in your backtrace, I don't see how you tripped over the assert, you did run in the release build crash. That won't change if I remove the assert now and the invariant is broken again, we just get a worse developer experience for debug compiles. Perhaps you use assertions for other things, I use them in most cases for the stuff you read for example here https://en.wikipedia.org/wiki/Assertion_(software_development) * Assertions during the development cycle I would be happy if that assertion could be proven during compile time as some static assertion, but static analysis will not do that for you with reasonable effort. That would actually be the nicest kind of assert there: no compile if we break this logic. -- You are receiving this mail because: You are watching all bug changes.
[kate] [Bug 408539] [KTextEditor] highlighting crash applying invalid format
https://bugs.kde.org/show_bug.cgi?id=408539 --- Comment #5 from RJVB --- Well, you kept the assert I tripped over, didn't you? Either you had your reasons for that, or that just proves one of my points above :) -- You are receiving this mail because: You are watching all bug changes.
[kate] [Bug 408539] [KTextEditor] highlighting crash applying invalid format
https://bugs.kde.org/show_bug.cgi?id=408539 --- Comment #4 from Christoph Cullmann --- I don't think that it makes sense to apply patches to our code for issues that got fixed just to change the coding style to your needs. But I can't stop people doing things they like. As your change will alter the line numbers in your code base, this will make backtraces from people using that version less useless for us, if we don't look at your code base (independent of the version they specify here). -- You are receiving this mail because: You are watching all bug changes.
[kate] [Bug 408539] [KTextEditor] highlighting crash applying invalid format
https://bugs.kde.org/show_bug.cgi?id=408539 --- Comment #3 from RJVB --- I can indeed no longer reproduce the crash with that commit in place (but will keep my own mod in place too). -- You are receiving this mail because: You are watching all bug changes.
[kate] [Bug 408539] [KTextEditor] highlighting crash applying invalid format
https://bugs.kde.org/show_bug.cgi?id=408539 --- Comment #2 from RJVB --- >And no, the assert is correct. >If we arrive here with a invalid iterator, all is broken, this shall no happen. Well, I don't agree with that. I have said it before and will continue to hammer it down: it's unprofessional to allow production builds to crash because of situations of which you have foreseen that they might happen. If the situation cannot be caught when it arrives an immediate abort is indeed the only solution rather than letting the software maybe crash at a later point. In all other situations thought should be given to users who might lose important data. Once there is sufficient proof that the situation can no longer arise the code can be cleaned up, but asserts have the nasty habit of remaining to clutter the code. I do agree that there should be a Q_ASSERT variant which makes it easy to execute a bit of graceful error handling. -- You are receiving this mail because: You are watching all bug changes.
[kate] [Bug 408539] [KTextEditor] highlighting crash applying invalid format
https://bugs.kde.org/show_bug.cgi?id=408539 Christoph Cullmann changed: What|Removed |Added Status|REPORTED|RESOLVED CC||cullm...@kde.org Resolution|--- |FIXED --- Comment #1 from Christoph Cullmann --- Thanks for the report with usable backtrace, considering the line numbers, I think this should be already fixed in master. Please try with master, the fix should be: commit e540262fb7230dfd40794d20c5d0e34dbc748b03 Author: Dāvis Mosāns Date: Sat Mar 16 16:40:24 2019 +0100 Don't crash on malformed syntax highlighting files Test Plan: 1. Create malformed syntax highlighting file with missing end tags 2. Place it in ~/.local/share/katepart5/syntax/ 3. Open Kate with file which uses that syntax highlighting Reviewers: cullmann, dhaumann Reviewed By: cullmann Subscribers: kwrite-devel, kde-frameworks-devel Tags: #kate, #frameworks Differential Revision: https://phabricator.kde.org/D19533 And no, the assert is correct. If we arrive here with a invalid iterator, all is broken, this shall no happen. We don't add early out for all cases of internal programming errors that shall be fixed (and got fixed). -- You are receiving this mail because: You are watching all bug changes.
