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
- Lint
Lint Skipped - Unit
Unit Tests Skipped
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 ↗ | (On Diff #51588) | 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 ↗ | (On Diff #51588) | 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.