Skip to content
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

Do not use globals for regex #10951

Merged
merged 2 commits into from
Jul 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 5 additions & 18 deletions src/compiler/crystal/codegen/codegen.cr
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ module Crystal
end
get_global class_var_global_name(node_exp.var), node_exp.type, node_exp.var
when Global
get_global node_exp.name, node_exp.type, node_exp.var
node.raise "BUG: there should be no use of global variables other than $~ and $?"
when Path
# Make sure the constant is initialized before taking a pointer of it
const = node_exp.target_const.not_nil!
Expand Down Expand Up @@ -1017,7 +1017,7 @@ module Crystal
when InstanceVar
instance_var_ptr context.type, target.name, llvm_self_ptr
when Global
get_global target.name, target_type, target.var
node.raise "BUG: there should be no use of global variables other than $~ and $?"
when ClassVar
read_class_var_ptr(target)
when Var
Expand Down Expand Up @@ -1143,15 +1143,7 @@ module Crystal
codegen_assign(var, value, node)
end
when Global
if value = node.value
request_value do
accept value
end

ptr = get_global var.name, var.type, var.var
assign ptr, var.type, value.type, @last
return false
end
node.raise "BUG: there should be no use of global variables other than $~ and $?"
when ClassVar
# This is the case of a class var initializer
initialize_class_var(var)
Expand Down Expand Up @@ -1208,18 +1200,13 @@ module Crystal
end

def visit(node : Global)
read_global node.name.to_s, node.type, node.var
node.raise "BUG: there should be no use of global variables other than $~ and $?"
end

def visit(node : ClassVar)
@last = read_class_var(node)
end

def read_global(name, type, real_var)
@last = get_global name, type, real_var
@last = to_lhs @last, type
end

def visit(node : InstanceVar)
read_instance_var node.type, context.type, node.name, llvm_self_ptr
end
Expand Down Expand Up @@ -2255,7 +2242,7 @@ module Crystal
end

def visit(node : ExpandableNode)
raise "BUG: #{node} at #{node.location} should have been expanded"
raise "BUG: #{node} (#{node.class}) at #{node.location} should have been expanded"
end

def visit(node : ASTNode)
Expand Down
4 changes: 0 additions & 4 deletions src/compiler/crystal/program.cr
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ module Crystal
# All symbols (:foo, :bar) found in the program
getter symbols = Set(String).new

# All global variables in the program ($foo, $bar), indexed by their name.
# The names includes the `$` sign.
getter global_vars = {} of String => MetaTypeVar

# Hash that prevents recursive splat expansions. For example:
#
# ```
Expand Down
15 changes: 14 additions & 1 deletion src/compiler/crystal/semantic/cleanup_transformer.cr
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ module Crystal

if target.is_a?(Path)
const = target.target_const.not_nil!
return node unless const.used?
return node if !const.used? || const.cleaned_up?

unless const.value.type?
node.raise "can't infer type of constant #{const} (maybe the constant refers to itself?)"
Expand All @@ -285,6 +285,7 @@ module Crystal
if target.is_a?(Path)
const = const.not_nil!
const.value = const.value.transform self
const.cleaned_up = true
end

if node.target == node.value
Expand All @@ -301,6 +302,18 @@ module Crystal
node
end

def transform(node : Path)
# Some constants might not have been cleaned up at this point because
# they don't have an explicit `Assign` node. One example is regex
# literals: a constant is created for them, but there's no `Assign` node.
if (const = node.target_const) && const.used? && !const.cleaned_up?
const.value = const.value.transform self
const.cleaned_up = true
end

node
end

private def void_lib_call?(node)
return unless node.is_a?(Call)

Expand Down
19 changes: 0 additions & 19 deletions src/compiler/crystal/semantic/exception.cr
Original file line number Diff line number Diff line change
Expand Up @@ -314,25 +314,6 @@ module Crystal
end

class Program
def undefined_global_variable(node, similar_name)
common = String.build do |str|
str << "can't infer the type of global variable '#{node.name}'"
if similar_name
str << '\n'
str << colorize(" (did you mean #{similar_name}?)").yellow.bold.to_s
end
end

msg = String.build do |str|
str << common
str << "\n\n"
str << undefined_variable_message("global", node.name)
str << "\n\n"
str << common
end
node.raise msg
end

def undefined_class_variable(node, owner, similar_name)
common = String.build do |str|
str << "can't infer the type of class variable '#{node.name}' of #{owner.devirtualize}"
Expand Down
43 changes: 15 additions & 28 deletions src/compiler/crystal/semantic/literal_expander.cr
Original file line number Diff line number Diff line change
Expand Up @@ -253,18 +253,16 @@ module Crystal
#
# /regex/flags
#
# To:
# To declaring a constant with this value (if not already declared):
#
# if temp_var = $some_global
# temp_var
# else
# $some_global = Regex.new("regex", Regex::Options.new(flags))
# end
# ```
# Regex.new("regex", Regex::Options.new(flags))
# ```
#
# That is, cache the regex in a global variable.
# and then reading from that constant.
# That is, we cache regex literals to avoid recompiling them all of the time.
#
# Only do this for regex literals that don't contain interpolation.
#
# If there's an interpolation, expand to: Regex.new(interpolation, flags)
def expand(node : RegexLiteral)
node_value = node.value
Expand All @@ -273,30 +271,19 @@ module Crystal
string = node_value.value

key = {string, node.options}
index = @regexes.index key
unless index
index = @regexes.size
@regexes << key
end

global_name = "$Regex:#{index}"
temp_name = @program.new_temp_var_name
index = @regexes.index(key) || @regexes.size
const_name = "$Regex:#{index}"

global_var = MetaTypeVar.new(global_name)
global_var.owner = @program
type = @program.nilable(@program.regex)
global_var.freeze_type = type
global_var.type = type
if index == @regexes.size
@regexes << key

# TODO: need to bind with nil_var for codegen, but shouldn't be needed
global_var.bind_to(@program.nil_var)
const_value = regex_new_call(node, StringLiteral.new(string).at(node))
const = Const.new(@program, @program, const_name, const_value)

@program.global_vars[global_name] = global_var
@program.types[const_name] = const
end

first_assign = Assign.new(Var.new(temp_name).at(node), Global.new(global_name).at(node)).at(node)
regex = regex_new_call(node, StringLiteral.new(string).at(node))
second_assign = Assign.new(Global.new(global_name).at(node), regex).at(node)
If.new(first_assign, Var.new(temp_name).at(node), second_assign).at(node)
Path.new(const_name)
else
regex_new_call(node, node_value)
end
Expand Down
76 changes: 3 additions & 73 deletions src/compiler/crystal/semantic/main_visitor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -423,22 +423,6 @@ module Crystal
class_var = lookup_class_var(var)
var.var = class_var
class_var.thread_local = true if thread_local
when Global
if @untyped_def
node.raise "declaring the type of a global variable must be done at the class level"
end

thread_local = check_class_var_annotations
if thread_local
global_var = @program.global_vars[var.name]
global_var.thread_local = true
end

if value = node.value
type_assign(var, value, node)
node.bind_to(var)
return false
end
else
raise "Bug: unexpected var type: #{var.class}"
end
Expand Down Expand Up @@ -586,39 +570,12 @@ module Crystal
node.bind_to expanded
node.expanded = expanded
else
visit_global node
node.raise "BUG: there should be no use of global variables other than $~ and $?"
end

false
end

def visit_global(node)
var = lookup_global_variable(node)

if first_time_accessing_meta_type_var?(var)
var_type = var.type?
if var_type && !var_type.includes_type?(program.nil)
node.raise "global variable '#{node.name}' is read here before it was initialized, rendering it nilable, but its type is #{var_type}"
end
var.bind_to program.nil_var
end

node.bind_to var
node.var = var
var
end

def lookup_global_variable(node)
var = program.global_vars[node.name]?
undefined_global_variable(node) unless var
var
end

def undefined_global_variable(node)
similar_name = lookup_similar_global_variable_name(node)
program.undefined_global_variable(node, similar_name)
end

def undefined_instance_variable(owner, node)
similar_name = lookup_similar_instance_variable_name(node, owner)
program.undefined_instance_variable(node, owner, similar_name)
Expand All @@ -637,14 +594,6 @@ module Crystal
end
end

def lookup_similar_global_variable_name(node)
Levenshtein.find(node.name) do |finder|
program.global_vars.each_key do |name|
finder.test(name)
end
end
end

def first_time_accessing_meta_type_var?(var)
return false if var.uninitialized?

Expand Down Expand Up @@ -924,26 +873,7 @@ module Crystal
end

def type_assign(target : Global, value, node)
thread_local = check_class_var_annotations

value.accept self

var = lookup_global_variable(target)

# If we are assigning to a global inside a method, make it nilable
# if this is the first time we are assigning to it, because
# the method might be called conditionally
if @typed_def && first_time_accessing_meta_type_var?(var)
var.bind_to program.nil_var
end

var.thread_local = true if thread_local
target.var = var

target.bind_to var

node.bind_to value
var.bind_to value
node.raise "BUG: there should be no use of global variables other than $~ and $?"
end

def type_assign(target : ClassVar, value, node)
Expand Down Expand Up @@ -2490,7 +2420,7 @@ module Crystal
when ClassVar
visit_class_var exp
when Global
visit_global exp
node.raise "BUG: there should be no use of global variables other than $~ and $?"
when Path
exp.accept self
if const = exp.target_const
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/crystal/types.cr
Original file line number Diff line number Diff line change
Expand Up @@ -3152,6 +3152,9 @@ module Crystal
property? used = false
property? visited = false

# Was this const's value cleaned up by CleanupTransformer yet?
property? cleaned_up = false

# Is this constant accessed with pointerof(...)?
property? pointer_read = false

Expand Down