[ruby-core:95977] [Ruby master Feature#16377] Regexp literals should be frozen

[ruby-core:95977] [Ruby master Feature#16377] Regexp literals should be frozen

[jean.boussier]

Issue #16377 has been reported by byroot (Jean Boussier).

----------------------------------------
Feature #16377: Regexp literals should be frozen
https://bugs.ruby-lang.org/issues/16377

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
The following script:

```ruby
def mutate
  re = /foo/
  state = re.instance_variable_get(:@state)
  re.instance_variable_set(:@state, state.to_i + 1)
  state
end

3.times do
  p mutate
end
```

Output this:

```
nil
1
2
```

IMHO, you shouldn't be able to mutate an unduplicated literal.

GitHub pull request: https://github.com/ruby/ruby/pull/2705



-- 
https://bugs.ruby-lang.org/

[ruby-core:96012] [Ruby master Feature#16377] Regexp literals should be frozen

[jean.boussier]

Issue #16377 has been updated by byroot (Jean Boussier).


Apparently it's a duplicate of https://bugs.ruby-lang.org/issues/8948

----------------------------------------
Feature #16377: Regexp literals should be frozen
https://bugs.ruby-lang.org/issues/16377#change-82850

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
The following script:

```ruby
def mutate
  re = /foo/
  state = re.instance_variable_get(:@state)
  re.instance_variable_set(:@state, state.to_i + 1)
  state
end

3.times do
  p mutate
end
```

Output this:

```
nil
1
2
```

IMHO, you shouldn't be able to mutate an unduplicated literal.

GitHub pull request: https://github.com/ruby/ruby/pull/2705



-- 
https://bugs.ruby-lang.org/

[ruby-core:96050] [Ruby master Feature#16377] Regexp literals should be frozen

[daniel]

Issue #16377 has been updated by Dan0042 (Daniel DeLorme).


I'm against.

There's no point to freezing objects left and right, just to satisfy some feeling that "immutable objects are good".

This goes back to the [grand design change of making ruby immutable by default](https://bugs.ruby-lang.org/issues/16153#note-6), and I am firmly in the camp of "no".

----------------------------------------
Feature #16377: Regexp literals should be frozen
https://bugs.ruby-lang.org/issues/16377#change-82899

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
The following script:

```ruby
def mutate
  re = /foo/
  state = re.instance_variable_get(:@state)
  re.instance_variable_set(:@state, state.to_i + 1)
  state
end

3.times do
  p mutate
end
```

Output this:

```
nil
1
2
```

IMHO, you shouldn't be able to mutate an unduplicated literal.

GitHub pull request: https://github.com/ruby/ruby/pull/2705



-- 
https://bugs.ruby-lang.org/

[ruby-core:96051] [Ruby master Feature#16377] Regexp literals should be frozen

[daniel]

Issue #16377 has been updated by Dan0042 (Daniel DeLorme).


Sorry, I completely missed the part about "unduplicated". I will reflect on writing replies too fast. m(_ _)m

----------------------------------------
Feature #16377: Regexp literals should be frozen
https://bugs.ruby-lang.org/issues/16377#change-82900

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
The following script:

```ruby
def mutate
  re = /foo/
  state = re.instance_variable_get(:@state)
  re.instance_variable_set(:@state, state.to_i + 1)
  state
end

3.times do
  p mutate
end
```

Output this:

```
nil
1
2
```

IMHO, you shouldn't be able to mutate an unduplicated literal.

GitHub pull request: https://github.com/ruby/ruby/pull/2705



-- 
https://bugs.ruby-lang.org/

[ruby-core:96204] [Ruby master Feature#16377] Regexp literals should be frozen

[daniel]

Issue #16377 has been updated by Dan0042 (Daniel DeLorme).


I really hope this does not go through.

Regexp literals have been "unduplicated" like this since at least 1.8 and we've never had problems. And now we should freeze them and introduce an incompatibility just for the sake of Communicating the Holy Gospel of Immutability? I don't find that a valid reason.

Additionaly, Regexp literals are not deduplicated in the same sense as frozen string literals; one `/abc/` is independant from another `/abc/` so we're not actually leaking "global" state. In the example above the state is local to the method but shared between invocations. Not sure how that should be called, but certainly not "global".

And what if the `mutate` behavior shown above is actually wanted? Sure it's a hack, but it's a bit like function-static variables in php.

Or, more realistically, what about something like this?

```ruby
class Regexp
  def analyze
    @analyze ||= RegexpAnalyzer.analyze_performance_issues(self)
  end
end
```

Freezing objects closes off possibilities when it's done by default, and should only be done when absolutely necessary.


----------------------------------------
Feature #16377: Regexp literals should be frozen
https://bugs.ruby-lang.org/issues/16377#change-83093

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
The following script:

```ruby
def mutate
  re = /foo/
  state = re.instance_variable_get(:@state)
  re.instance_variable_set(:@state, state.to_i + 1)
  state
end

3.times do
  p mutate
end
```

Output this:

```
nil
1
2
```

IMHO, you shouldn't be able to mutate an unduplicated literal.

GitHub pull request: https://github.com/ruby/ruby/pull/2705



-- 
https://bugs.ruby-lang.org/

[ruby-core:96211] [Ruby master Feature#16377] Regexp literals should be frozen

[jean.boussier]

Issue #16377 has been updated by byroot (Jean Boussier).


>  just for the sake of Communicating the Holy Gospel of Immutability?

Please put ideological thoughts on me, that's not a good basis for debate.

> Regexp literals are not deduplicated in the same sense as frozen string literals; one /abc/ is independant from another /abc/

And if they were frozen, they could be.

> so we're not actually leaking "global" state

On a particular callsite it does.

> Or, more realistically, what about something like this?

```ruby
class Regexp
  @@analyses = {}.compare_by_identity

  def analyze
    @@analyses[self] ||= RegexpAnalyzer.analyze_performance_issues(self)
  end
end
```

----------------------------------------
Feature #16377: Regexp literals should be frozen
https://bugs.ruby-lang.org/issues/16377#change-83103

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
The following script:

```ruby
def mutate
  re = /foo/
  state = re.instance_variable_get(:@state)
  re.instance_variable_set(:@state, state.to_i + 1)
  state
end

3.times do
  p mutate
end
```

Output this:

```
nil
1
2
```

IMHO, you shouldn't be able to mutate an unduplicated literal.

GitHub pull request: https://github.com/ruby/ruby/pull/2705



-- 
https://bugs.ruby-lang.org/

[ruby-core:96216] [Ruby master Feature#16377] Regexp literals should be frozen

[daniel]

Issue #16377 has been updated by Dan0042 (Daniel DeLorme).


> Please don't put ideological thoughts on me, that's not a good basis for debate.

Oh no, I'm not. Sorry for the misunderstanding. I was mostly replying to comments in #8948, but I ended up posting here because this is the ticket that is linked in #16393 DevelopersMeeting20191220Japan.

Yes, of course when one way is blocked there's always an alternate way of doing things. But the point is that if someone is *currently* relying on Regexp literals not being frozen, this change will break their code. Given that the benefit is close to zero, I don't think it's responsible to force anyone to bear that cost. I'm very conservative about backward compatibility.

----------------------------------------
Feature #16377: Regexp literals should be frozen
https://bugs.ruby-lang.org/issues/16377#change-83108

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
The following script:

```ruby
def mutate
  re = /foo/
  state = re.instance_variable_get(:@state)
  re.instance_variable_set(:@state, state.to_i + 1)
  state
end

3.times do
  p mutate
end
```

Output this:

```
nil
1
2
```

IMHO, you shouldn't be able to mutate an unduplicated literal.

GitHub pull request: https://github.com/ruby/ruby/pull/2705



-- 
https://bugs.ruby-lang.org/

[ruby-core:96482] [Ruby master Feature#16377] Regexp literals should be frozen

[mame]

Issue #16377 has been updated by mame (Yusuke Endoh).

Assignee set to mame (Yusuke Endoh)

At the previous dev meeting, matz said that let's give it a try :-)

For the record: `Regexp.new` should continue to return unfrozen Regexp instance.

I'll review the pull request.

----------------------------------------
Feature #16377: Regexp literals should be frozen
https://bugs.ruby-lang.org/issues/16377#change-83411

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Assignee: mame (Yusuke Endoh)
* Target version: 2.8
----------------------------------------
The following script:

```ruby
def mutate
  re = /foo/
  state = re.instance_variable_get(:@state)
  re.instance_variable_set(:@state, state.to_i + 1)
  state
end

3.times do
  p mutate
end
```

Output this:

```
nil
1
2
```

IMHO, you shouldn't be able to mutate an unduplicated literal.

GitHub pull request: https://github.com/ruby/ruby/pull/2705



-- 
https://bugs.ruby-lang.org/

[ruby-core:96860] [Ruby master Feature#16377] Regexp literals should be frozen

[ko1]

Issue #16377 has been updated by ko1 (Koichi Sasada).


Could you add a NEWS entry?

----------------------------------------
Feature #16377: Regexp literals should be frozen
https://bugs.ruby-lang.org/issues/16377#change-83871

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Assignee: mame (Yusuke Endoh)
* Target version: 2.8
----------------------------------------
The following script:

```ruby
def mutate
  re = /foo/
  state = re.instance_variable_get(:@state)
  re.instance_variable_set(:@state, state.to_i + 1)
  state
end

3.times do
  p mutate
end
```

Output this:

```
nil
1
2
```

IMHO, you shouldn't be able to mutate an unduplicated literal.

GitHub pull request: https://github.com/ruby/ruby/pull/2705



-- 
https://bugs.ruby-lang.org/