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
Details
Details
- Reviewers
vkrause cullmann - Commits
- R216:152968754edc: prevent assertion in regex load
Diff Detail
Diff Detail
- Repository
- R216 Syntax Highlighting
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
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.
| src/lib/rule.cpp | ||
|---|---|---|
| 579–582 | 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(); | |
| src/lib/rule.cpp | ||
|---|---|---|
| 579–582 | Unconditionally running the isValid check is to expensive. At least according to the comment above the code, but I'haven't checked this statement. | |
Comment Actions
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.