prevent assertion in regex load

Authored by davschul on Feb 13 2019, 2:02 PM.



Instead of crashing the complete application with an assert on a malformed
regex use the logging category to inform users when such a regex is found.
This way the check can also be enabled for release builds, and don't have to
run unnecessarily for debug builds

Diff Detail

R216 Syntax Highlighting
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
davschul created this revision.Feb 13 2019, 2:02 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptFeb 13 2019, 2:02 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
davschul requested review of this revision.Feb 13 2019, 2:02 PM

I think that makes a lot of sense. Maybe we should even always print the warning, not only in debug mode? See comment below...

PS: When submitting patches without phabricator's arc tool, could you add more diff context, like git diff -U99 .... Then reviewing is much more simple.


I even wonder whether this should be always printed, i.e.:

const bool isValid = m_regexp.isValid();
if (!isValid)
    qCWarning(...) << "Invalid regexp:" << m_regexp.pattern();
return isValid && !m_regexp.pattern().isEmpty();
davschul added inline comments.Feb 14 2019, 6:41 AM

Unconditionally running the isValid check is to expensive. At least according to the comment above the code, but I'haven't checked this statement.

davschul updated this revision to Diff 51643.Feb 14 2019, 6:44 AM

added additional context to the diff

cullmann accepted this revision.Feb 14 2019, 7:05 AM
cullmann added a subscriber: cullmann.

I assume Volker profiled it.
I think it is ok to just check if the logging is on.
99% of the syntax definitions are anyways bundled and are checked by the indexer for such obvious faults.

This revision is now accepted and ready to land.Feb 14 2019, 7:05 AM

I can push that for you, which user to use? "David Schulz" <paste your mail?>

David Schulz

This revision was automatically updated to reflect the committed changes.