Skip to content

Clearer error message when regex exceeds space limit #1428

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

Merged
merged 1 commit into from
Jul 10, 2017

Conversation

rimmington
Copy link
Contributor

While investigating why nixpkgs-mozilla stopped working on our Hydra instance, I discovered that there's an upper limit to the size of a regular expression. This took quite a while, because Nix reported "invalid regular expression" when the limit was exceeded but the expression seemed syntactically valid.

This PR improves the error message in this specific case. I tried using e.what() but the resulting message was very unhelpful (literally regex_error).

throw EvalError("invalid regular expression ‘%s’, at %s", re, pos);
} catch (std::regex_error &e) {
if (e.code() == std::regex_constants::error_space) {
throw EvalError("regular expression exceeds _GLIBCXX_REGEX_STATE_LIMIT ‘%s’, at %s", re, pos);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nitpick: _GLIBCXX_REGEX_STATE_LIMIT is libstdc++ specific. Maybe something like "insufficient memory for regular expression ..."?

@rimmington rimmington force-pushed the clearer-regex-space-error branch from f5fe993 to 6b5532d Compare July 9, 2017 23:18
@rimmington
Copy link
Contributor Author

@edolstra I've moved mention of the specific macro to a comment. I'm hesitant to use a phrase like "insufficient memory", because (at least in libstdc++) the limit isn't related to the amount of memory available to the process and so might lead people down the wrong path. The wording is still a bit awkward.

@rimmington rimmington force-pushed the clearer-regex-space-error branch from 6b5532d to 17bb00d Compare July 9, 2017 23:36
@edolstra edolstra merged commit 1762b96 into NixOS:master Jul 10, 2017
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

3 participants