Compiler: mark LLVM return type as Nil when program ends with no typed ASTNode #3275

maiha · 2016-09-07T15:45:02Z

Compiler crashes when the crystal code ends with no typed ASTNode such as @[Foo].

test.cr

@[Foo]

run

% crystal -v
Crystal 0.19.0 [dcfb2b6] (2016-09-02)

% crystal test.cr
(...snip...)
@[Foo]
` at  has no type
[8917383] *CallStack::unwind:Array(Pointer(Void)) +87
[8917274] *CallStack#initialize:Array(Pointer(Void)) +10
[8917226] *CallStack::new:CallStack +42
[8756568] *raise<Exception>:NoReturn +24
[8756542] ???
[13486612] *Crystal::ASTNode+ +148
[21386979] *Crystal::CodeGenVisitor#initialize<Crystal::Program, Crystal::AS
TNode+, (Array(String) | Bool | Nil), Bool, LLVM::Module, Bool>:Bool +339
[21386606] *Crystal::CodeGenVisitor::new:single_module:debug:llvm_mod:expose
_crystal_main<Crystal::Program, Crystal::ASTNode+, (Array(String) | Bool | N
il), Bool, LLVM::Module, Bool>:Crystal::CodeGenVisitor +222
...

I think this will occur in all crystal versions because I got same errors in 0.18.7.

PR

This PR uses nil for LLVM return type when typeless node appears in the last sentence.

% ./bin/crystal -v
Using compiled compiler at .build/crystal
Crystal 0.19.0+24 [4e00257] (2016-09-08)

% ./bin/crystal test.cr
Using compiled compiler at .build/crystal
# (→ no errors )

Although it's nice that the attributes should be typed correctly,
we also need this fix because compiler must not crash in any cases we don't predict.

make clean crystal spec has been passed.

Thanks.

asterite · 2016-09-08T12:31:25Z

src/compiler/crystal/codegen/codegen.cr

@@ -131,8 +131,8 @@ module Crystal
      @abi = @program.target_machine.abi
      @llvm_typer = @program.llvm_typer
      @llvm_id = LLVMId.new(@program)
-      @main_ret_type = node.type
-      ret_type = @llvm_typer.llvm_return_type(node.type)
+      @main_ret_type = node.type? ? node.type : @program.nil_type


Please use node.type? || @program.nil_type

asterite · 2016-09-08T12:32:06Z

Thank you, looks good! I just made a small comment. Maybe a spec would be nice, but I think it breaks in the spec code because it assumes the node will have a type, so that would need to change too...

…d ASTNode

maiha · 2016-09-09T00:35:10Z

Hi asterite!

Thank you for your advice. I fixed about it and pushed again.

I think it breaks in the spec code because it assumes the node will have a type

Well, could you tell me more about this spec file?
Although I've already passed make clean crystal spec, it would be I missed some steps
for all tests because it's first time to test compiler for me.

I'd like to try to fix the spec too.

Thanks!

asterite · 2016-09-09T14:52:31Z

@maiha Don't worry, the change is OK. We could maybe need a spec like codegen("@[Foo]") and see that it doesn't break. In any case these programs are not very useful, but it's better if the compiler doesn't break.

Thank you!

maiha force-pushed the typeless-as-nil branch from 2bb1cd9 to 3f7c200 Compare September 8, 2016 00:20

asterite reviewed Sep 8, 2016
View reviewed changes

Compiler: mark LLVM return type as Nil when program ends with no type…

77c4cea

…d ASTNode

maiha force-pushed the typeless-as-nil branch from 3f7c200 to 77c4cea Compare September 9, 2016 00:27

asterite added this to the 0.19.1 milestone Sep 9, 2016

asterite merged commit a7bc280 into crystal-lang:master Sep 9, 2016

maiha deleted the typeless-as-nil branch September 9, 2016 15:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compiler: mark LLVM return type as Nil when program ends with no typed ASTNode #3275

Compiler: mark LLVM return type as Nil when program ends with no typed ASTNode #3275

maiha commented Sep 7, 2016

asterite Sep 8, 2016

asterite commented Sep 8, 2016

maiha commented Sep 9, 2016

asterite commented Sep 9, 2016

Compiler: mark LLVM return type as Nil when program ends with no typed ASTNode #3275

Compiler: mark LLVM return type as Nil when program ends with no typed ASTNode #3275

Conversation

maiha commented Sep 7, 2016

test.cr

run

PR

asterite Sep 8, 2016

Choose a reason for hiding this comment

asterite commented Sep 8, 2016

maiha commented Sep 9, 2016

asterite commented Sep 9, 2016