Skip to content

Commit

Permalink
Compiler: don't use global for regex literals
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite committed Jul 15, 2021
1 parent af095d7 commit 5d34b3e
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 143 deletions.
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

0 comments on commit 5d34b3e

Please sign in to comment.