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

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

Merged
merged 1 commit into from Sep 9, 2016

Conversation

maiha
Copy link
Contributor

@maiha maiha commented Sep 7, 2016

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.

@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@asterite
Copy link
Member

asterite commented Sep 8, 2016

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...

@maiha
Copy link
Contributor Author

maiha commented Sep 9, 2016

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
Copy link
Member

asterite commented Sep 9, 2016

@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!

@asterite asterite added this to the 0.19.1 milestone Sep 9, 2016
@asterite asterite merged commit a7bc280 into crystal-lang:master Sep 9, 2016
@maiha maiha deleted the typeless-as-nil branch September 9, 2016 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants