update for frozen string literals #1101
Conversation
Are these test related to the Sinatra repo? any situation where the failure can be reproduced? The master branch looks green. |
|
Yes, it was Sinatra tests that were failing when using default frozen string literals - specifically attempting to concat to frozen strings, see the changes in base.rb. They don't fail in master because it's not defaulting to frozen string literals. |
| @@ -1,3 +1,4 @@ | |||
| # frozen_string_literal: true | |||
There was a problem hiding this comment.
This isn't needed for the one string allocated here "sinatra/main" is an acceptable allocation that I doubt will be repeated enough to affect performance
|
I also don't want to litter the test files with this comment, as it's only necessary for runtime performance |
| @@ -842,6 +842,7 @@ def compile_template(engine, data, options, views) | |||
| body, path, line = settings.templates[data] | |||
| if body | |||
| body = body.call if body.respond_to?(:call) | |||
| body = body.dup if body.frozen? | |||
There was a problem hiding this comment.
Can you provide a case where body is actually immutable from here? Maybe an empty request?
|
It'd be nice if frozen string literals were supported in rack-protection as well. I could open a separate PR for that if it'd be helpful. |
Frozen string literals - magic comment in all files and changes for tests to pass