Issue #19694 has been updated by Eregon (Benoit Daloze). byroot (Jean Boussier) wrote in #note-5:
I assume you could have an internal regexp object that is deduplicated and immutable, and the actual `Regexp` object that is exposed could be a tuple of `(internal_regexp, timeout)`.
Not really, it would be very messy and have a performance cost. The reason is we want to embed Regexp objects in the context-independent AST (for persisting JITed code), and so it must not refer to context-specific state. We could have two types, one for immutable regexps and one for mutable regexps, we do this for String, but it is a nightmare and is the only type I will do that for. Similarly, Ractor wants to be able to use Regexp objects on any Ractor, the requirement for that is that as many Regexp as possible are immutable. Probably we should make all Regexps immutable in CRuby too for consistency (both across Rubies and between literal/non-literal) and it would benefit Ractor. ---------------------------------------- Feature #19694: Add Regexp#timeout= setter https://bugs.ruby-lang.org/issues/19694#change-103319 * Author: aharpole (Aaron Harpole) * Status: Open * Priority: Normal ---------------------------------------- # Abstract In addition to allowing for a Regexp timeout to be set on individual instances by setting a `timeout` argument in `Regexp.new`, I'm proposing that we also allow setting the timeout on Regexp objects with a `#timeout=` setter. # Background To be able to roll out a global Regexp timeout for a large application, there are inevitably some individual regexes for which a different timeout is appropriate. While the `timeout` keyword argument was added to `Regexp.new`, this isn't always a viable option. In the case of regex literal syntax (`/ab*/` or `%r{ab*}`, for instance), it's not possible to set a timeout at all right now without converting to `Regexp.new`, which may be awkward depending on the contents of the regex. It also is desirable from time to time to be able to set a timeout for a regex object after it's been initialized. Finally, because we offer a `Regexp#timeout` getter, for consistency it would be nice to also offer a setter. The introduction of a `Regexp#timeout=` setter was mentioned as a possible way to set individual timeouts in https://bugs.ruby-lang.org/issues/19104#Specification. # Proposal I propose that we add the method `Regexp#timeout=`. It works the same way the `timeout` argument works in `Regexp.new`, taking either a float or nil. This makes it relatively easy to add timeouts to specific regex literals (regex literals are frozen by default so you do have to `dup` them first): ``` emoji_filter_pattern = %r{ (?<!#{Regexp.quote(ZERO_WIDTH_JOINER)}) #{EmojiFilter.unicodes_pattern} (?!#{Regexp.union(EmojiFilter::MODIFIER_CHAR_MAP.keys.map { |k| Regexp.quote k })}) }x.dup emoji_filter_pattern.timeout = 1.0 emoji_filter_pattern.freeze ``` # Implementation This setter has been implemented in https://github.com/ruby/ruby/pull/7847. # Evaluation It's just a setter, so pretty straightforward in terms of implementation and use. # Discussion It's worth considering other options for overriding `Regexp.timeout`. I'd love to see something like the following for overriding regexp timeouts as well: ``` Regexp.timeout = 1.0 Regexp.with_timeout(5.0) do evaluate_slower_regexes end ``` It's possible to implement something like `Regexp.with_timeout` but it's not thread-safe by default since it would involve overwriting `Regexp.timeout`. # Summary Regexp instances have a getter for timeout, and adding a corresponding setter adds consistency and will make it easier for developers to adopt adding a global `Regexp.timeout` by making it simpler to adjust timeouts on a regex by regex basis. It's a minor change but the added consistency and flexibility help us optimize for developer happiness. -- https://bugs.ruby-lang.org/