Better way to do {{yield}} replacements in macros (faster and preserv…

…ing original location)

spec/compiler/type_inference/macro_spec.cr

@@ -289,4 +289,20 @@ describe "Type inference: macro" do
       ),
       "Error in line 7"
   end
+
+  it "transforms with {{yield}} and call" do
+    assert_type(%(
+      macro foo
+        bar({{yield}})
+      end
+
+      def bar(value)
+        value
+      end
+
+      foo do
+        1 + 2
+      end
+      )) { int32 }
+  end
 end

spec/spec_helper.cr

@@ -103,6 +103,7 @@ def assert_macro(macro_args, macro_body, expected)
   program = Program.new
   call = Call.new(nil, "", yield program)
   result = program.expand_macro program, a_macro, call
+  result = result.source
   result = result[0 .. -2] if result.ends_with?(';')
   result.should eq(expected)
 end

src/compiler/crystal/macros/macros.cr

@@ -35,7 +35,7 @@ module Crystal
       end
 
       begin
-        generated_source = @program.expand_macro owner, target_def.body
+        expanded_macro = @program.expand_macro owner, target_def.body
       rescue ex : Crystal::Exception
         target_def.raise "expanding macro", ex
       end
@@ -49,7 +49,7 @@ module Crystal
 
       arg_names = target_def.args.map(&.name)
 
-      generated_nodes = parse_macro_source(generated_source, the_macro, target_def, arg_names.to_set) do |parser|
+      generated_nodes = parse_macro_source(expanded_macro, the_macro, target_def, arg_names.to_set) do |parser|
         parser.parse_to_def(target_def)
       end
 
@@ -64,38 +64,54 @@ module Crystal
       target_def.body = generated_nodes
     end
 
-    def parse_macro_source(generated_source, the_macro, node, vars, inside_def = false)
-      parse_macro_source generated_source, the_macro, node, vars, inside_def, &.parse
+    def parse_macro_source(expanded_macro, the_macro, node, vars, inside_def = false)
+      parse_macro_source expanded_macro, the_macro, node, vars, inside_def, &.parse
     end
 
-    def parse_macro_source(generated_source, the_macro, node, vars, inside_def = false)
+    def parse_macro_source(expanded_macro, the_macro, node, vars, inside_def = false)
+      generated_source = expanded_macro.source
       begin
         parser = Parser.new(generated_source, [vars.dup])
         parser.filename = VirtualFile.new(the_macro, generated_source, node.location)
         parser.visibility = node.visibility
         parser.def_nest = 1 if inside_def
-        normalize(yield parser)
+        generated_node = yield parser
+        if yields = expanded_macro.yields
+          generated_node = generated_node.transform(YieldsTransformer.new(yields))
+        end
+        normalize(generated_node)
       rescue ex : Crystal::SyntaxException
         node.raise "macro didn't expand to a valid program, it expanded to:\n\n#{"=" * 80}\n#{"-" * 80}\n#{generated_source.lines.to_s_with_line_numbers}\n#{"-" * 80}\n#{ex.to_s_with_source(generated_source)}\n#{"=" * 80}"
       end
     end
   end
 
   class MacroExpander
+    # When a macro is expanded the result is a source code to be parsed.
+    # When a macro contains `{{yield}}`, instead of transforming the yielded
+    # node to a String, which would cause loss of location information (which could
+    # be added with a loc pragma, but it would be slow) a placeholder is created
+    # and later must be replaced. The mapping of placeholders is the `yields` property
+    # of this record. What must be replaced are argless calls whose name appear in this
+    # `yields` hash.
+    record ExpandedMacro, source, yields
+
     def initialize(@mod)
       @cache = {} of String => String
     end
 
     def expand(scope : Type, a_macro, call)
       visitor = MacroVisitor.new self, @mod, scope, a_macro, call
       a_macro.body.accept visitor
-      visitor.to_s
+      source = visitor.to_s
+      ExpandedMacro.new source, visitor.yields
     end
 
     def expand(scope : Type, node)
       visitor = MacroVisitor.new self, @mod, scope, node.location
       node.accept visitor
-      visitor.to_s
+      source = visitor.to_s
+      ExpandedMacro.new source, visitor.yields
     end
 
     def run(filename, args)
@@ -132,6 +148,7 @@ module Crystal
 
     class MacroVisitor < Visitor
       getter last
+      getter yields
 
       def self.new(expander, mod, scope, a_macro : Macro, call)
         vars = {} of String => ASTNode
@@ -206,7 +223,13 @@ module Crystal
         node.exp.accept self
 
         if node.output
-          @last.to_s(@str, include_location: node.exp.is_a?(Yield))
+          if node.exp.is_a?(Yield) && !@last.is_a?(Nop)
+            var_name = @mod.new_temp_var_name
+            yields = @yields ||= {} of String => ASTNode
+            yields[var_name] = @last
+            @last = Var.new(var_name)
+          end
+          @last.to_s(@str)
         end
 
         false
@@ -680,4 +703,13 @@ module Crystal
       end
     end
   end
+
+  class YieldsTransformer < Transformer
+    def initialize(@yields)
+    end
+
+    def transform(node : Call)
+      @yields[node.name]? || super
+    end
+  end
 end

src/compiler/crystal/semantic/method_missing.cr

@@ -63,8 +63,8 @@ module Crystal
       a_def = Def.new(signature.name, args_nodes_names.map { |name| Arg.new(name) })
 
       fake_call = Call.new(nil, "method_missing", [name_node, args_node, block_node] of ASTNode)
-      generated_source = program.expand_macro self, method_missing, fake_call
-      generated_nodes = program.parse_macro_source(generated_source, method_missing, method_missing, args_nodes_names) do |parser|
+      expanded_macro = program.expand_macro self, method_missing, fake_call
+      generated_nodes = program.parse_macro_source(expanded_macro, method_missing, method_missing, args_nodes_names) do |parser|
         parser.parse_to_def(a_def)
       end
 

src/compiler/crystal/semantic/type_inference.cr

@@ -1080,12 +1080,12 @@ module Crystal
 
     def expand_macro(the_macro, node)
       begin
-        generated_source = yield
+        expanded_macro = yield
       rescue ex : Crystal::Exception
         node.raise "expanding macro", ex
       end
 
-      generated_nodes = @mod.parse_macro_source(generated_source, the_macro, node, Set.new(@vars.keys), inside_def: !!@typed_def)
+      generated_nodes = @mod.parse_macro_source(expanded_macro, the_macro, node, Set.new(@vars.keys), inside_def: !!@typed_def)
       generated_nodes.accept self
       generated_nodes
     end

src/compiler/crystal/syntax/location.cr

@@ -23,16 +23,5 @@ module Crystal
     def to_s(io)
       io << filename << ":" << line_number << ":" << column_number
     end
-
-    def to_s_as_comment(io)
-      io << %(#<loc:")
-      io << filename
-      io << '"'
-      io << ','
-      io << line_number
-      io << ','
-      io << column_number
-      io << '>'
-    end
   end
 end

src/compiler/crystal/syntax/to_s.cr

@@ -7,14 +7,14 @@ module Crystal
       to_s(io)
     end
 
-    def to_s(io, include_location = false)
-      visitor = ToSVisitor.new(io, include_location)
+    def to_s(io)
+      visitor = ToSVisitor.new(io)
       self.accept visitor
     end
   end
 
   class ToSVisitor < Visitor
-    def initialize(@str = StringIO.new, @include_location = false)
+    def initialize(@str = StringIO.new)
       @indent = 0
       @inside_macro = false
     end
@@ -232,8 +232,6 @@ module Crystal
     end
 
     def visit(node : Call)
-      output_location(node)
-
       if node.name == "`"
         visit_backtick(node.args[0])
         return false
@@ -1272,16 +1270,6 @@ module Crystal
       @str << newline
     end
 
-    def output_location(node)
-      return unless @include_location
-
-      location = node.location
-      return unless location.is_a?(Location)
-
-      @str << ' '
-      location.to_s_as_comment(@str)
-    end
-
     def to_s
       @str.to_s
     end