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.

Sign up for GitHub

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

Jump to bottom

Compiler: respect C ABI for procs and top-level methods that involve extern structs #3311

Merged
merged 1 commit into from
Sep 15, 2016
Merged

Compiler: respect C ABI for procs and top-level methods that involve extern structs #3311

merged 1 commit into from
Sep 15, 2016
+608 −39

Conversation

asterite
Copy link
Member

Fixes #605

@BlaXpirit Is there any chance you can test this change in some of your projects? The original snippet of #605 doesn't compile anymore, though I made a few tests that should cover that.

For now this works for procs and top-level methods (if you for example do ->foo(s : SomeStruct)), but it doesn't work for class methods. I might fix that later.

@asterite asterite added this to the 0.19.2 milestone Sep 15, 2016
@asterite asterite force-pushed the feature/fix_proc_extern_structs branch from 80d7f06 to ae026a8 Compare September 15, 2016 02:15
@asterite
Copy link
Member Author

Hm, I think this doesn't work for procs that only return C structs, I'll fix it soon.

@asterite asterite force-pushed the feature/fix_proc_extern_structs branch from ae026a8 to b05936a Compare September 15, 2016 02:59
@asterite
Copy link
Member Author

Done! I think this should cover a few cases that I missed before.

@oprypin
Copy link
Member

oprypin commented Sep 15, 2016

@asterite, I have bad news. Most of my usages still break.

I've updated the information about the dependency, and this snippet works correctly now, while the other one still gives the same wrong output.

My other library seems to give bad results so far, I'll try to gather some details.

@asterite asterite force-pushed the feature/fix_proc_extern_structs branch from b05936a to 56bc69d Compare September 15, 2016 13:12
@asterite
Copy link
Member Author

asterite commented Sep 15, 2016 •
edited
Loading

@BlaXpirit Got it! I was missing the case where the proc returned nil (nil can't be used as an argument, but yes as a return type, so I had a bug in the logic). I tried your second snippet and it now works well, it says "called with Pointer(Void)@0x123".

Can you try again now, also with other libs and code? If something doesn't work it should be very easy to make it work now (the core logic for this is already implemented)

@oprypin
Copy link
Member

oprypin commented Sep 15, 2016

Yes, the original samples work now.

But that just broke something in my other library (no idea where)

Index out of bounds
[9221367] *CallStack::unwind:Array(Pointer(Void)) +87
[9221258] *CallStack#initialize:Array(Pointer(Void)) +10
[9221210] *CallStack::new:CallStack +42
[9076968] *raise<IndexError>:NoReturn +24
[22354066] *LLVM::ParameterCollection#[]<Int32>:LLVM::Value +98
[22027647] *Crystal::CodeGenVisitor#create_local_copy_of_fun_args<Crystal::Def+, Crystal::Type+, Array(Crystal::Arg), Bool, Bool>:Array(Crystal::Arg) +479
[22016224] *Crystal::CodeGenVisitor#codegen_fun<String, Crystal::Def+, Crystal::Type+, Bool, LLVM::Module, Bool, Bool>:LLVM::Function +1968
[22014239] *Crystal::CodeGenVisitor#codegen_fun:fun_module:is_fun_literal:is_closure<String, Crystal::Def+, Crystal::Type+, LLVM::Module, Bool, Bool>:LLVM::Function +111
[22012411] *Crystal::CodeGenVisitor#visit<Crystal::ProcLiteral>:Bool +235
[14162868] *Crystal::ASTNode+ +1556
[22092551] *Crystal::CodeGenVisitor#codegen_call_with_block_as_fun_literal<Crystal::Call, Crystal::ASTNode+, Crystal::Type+, Array(LLVM::Value)>:LLVM::Value +87
[22068568] *Crystal::CodeGenVisitor#visit<Crystal::Call>:Bool +600
[14166660] *Crystal::ASTNode+ +5348
[21978017] *Crystal::CodeGenVisitor#accept<Crystal::ASTNode+>:Nil +17
[22035275] *Crystal::CodeGenVisitor#visit<Crystal::Expressions>:Bool +251
[14163064] *Crystal::ASTNode+ +1752
[21978017] *Crystal::CodeGenVisitor#accept<Crystal::ASTNode+>:Nil +17
[22016305] *Crystal::CodeGenVisitor#codegen_fun<String, Crystal::Def+, Crystal::Type+, Bool, LLVM::Module, Bool, Bool>:LLVM::Function +2049
[22034865] *Crystal::CodeGenVisitor#codegen_fun<String, Crystal::Def+, Crystal::Type+>:LLVM::Function +113
[22033819] *Crystal::CodeGenVisitor#target_def_fun<Crystal::Def+, Crystal::Type+>:LLVM::Function +379
[22104243] *Crystal::CodeGenVisitor#codegen_call<Crystal::Call, Crystal::Def+, Crystal::Type+, Array(LLVM::Value)>:(Bool | LLVM::Value | Nil) +323
[22068515] *Crystal::CodeGenVisitor#visit<Crystal::Call>:Bool +547
[14166660] *Crystal::ASTNode+ +5348
[21978017] *Crystal::CodeGenVisitor#accept<Crystal::ASTNode+>:Nil +17
[22038450] *Crystal::CodeGenVisitor#visit<Crystal::Assign>:(Bool | Nil) +818
[14165037] *Crystal::ASTNode+ +3725
[21978017] *Crystal::CodeGenVisitor#accept<Crystal::ASTNode+>:Nil +17
[22035275] *Crystal::CodeGenVisitor#visit<Crystal::Expressions>:Bool +251
[14163064] *Crystal::ASTNode+ +1752
[21978017] *Crystal::CodeGenVisitor#accept<Crystal::ASTNode+>:Nil +17
[22016305] *Crystal::CodeGenVisitor#codegen_fun<String, Crystal::Def+, Crystal::Type+, Bool, LLVM::Module, Bool, Bool>:LLVM::Function +2049
[22014239] *Crystal::CodeGenVisitor#codegen_fun:fun_module:is_fun_literal:is_closure<String, Crystal::Def+, Crystal::Type+, LLVM::Module, Bool, Bool>:LLVM::Function +111
[22012411] *Crystal::CodeGenVisitor#visit<Crystal::ProcLiteral>:Bool +235
[14162868] *Crystal::ASTNode+ +1556
[22092551] *Crystal::CodeGenVisitor#codegen_call_with_block_as_fun_literal<Crystal::Call, Crystal::ASTNode+, Crystal::Type+, Array(LLVM::Value)>:LLVM::Value +87
[22068568] *Crystal::CodeGenVisitor#visit<Crystal::Call>:Bool +600
[14166660] *Crystal::ASTNode+ +5348
[21978017] *Crystal::CodeGenVisitor#accept<Crystal::ASTNode+>:Nil +17
[22035275] *Crystal::CodeGenVisitor#visit<Crystal::Expressions>:Bool +251
[14163064] *Crystal::ASTNode+ +1752
[21978017] *Crystal::CodeGenVisitor#accept<Crystal::ASTNode+>:Nil +17
[22016305] *Crystal::CodeGenVisitor#codegen_fun<String, Crystal::Def+, Crystal::Type+, Bool, LLVM::Module, Bool, Bool>:LLVM::Function +2049
[22014239] *Crystal::CodeGenVisitor#codegen_fun:fun_module:is_fun_literal:is_closure<String, Crystal::Def+, Crystal::Type+, LLVM::Module, Bool, Bool>:LLVM::Function +111
[22012411] *Crystal::CodeGenVisitor#visit<Crystal::ProcLiteral>:Bool +235
[14162868] *Crystal::ASTNode+ +1556
[22092551] *Crystal::CodeGenVisitor#codegen_call_with_block_as_fun_literal<Crystal::Call, Crystal::ASTNode+, Crystal::Type+, Array(LLVM::Value)>:LLVM::Value +87
[22068568] *Crystal::CodeGenVisitor#visit<Crystal::Call>:Bool +600
[14166660] *Crystal::ASTNode+ +5348
[21978017] *Crystal::CodeGenVisitor#accept<Crystal::ASTNode+>:Nil +17
[22035275] *Crystal::CodeGenVisitor#visit<Crystal::Expressions>:Bool +251
[14163064] *Crystal::ASTNode+ +1752
[21978219] *Crystal::CodeGenVisitor#visit<Crystal::FileNode>:Bool +171
[14161704] *Crystal::ASTNode+ +392
[21978017] *Crystal::CodeGenVisitor#accept<Crystal::ASTNode+>:Nil +17
[22035275] *Crystal::CodeGenVisitor#visit<Crystal::Expressions>:Bool +251
[14163064] *Crystal::ASTNode+ +1752
[10307868] *Crystal::Program#codegen<Crystal::ASTNode+, (Array(String) | Bool | Nil), Bool, LLVM::Module, Bool>:Hash(String, LLVM::Module) +124
[10307734] *Crystal::Program#codegen:debug:single_module:expose_crystal_main<Crystal::ASTNode+, Bool, (Array(String) | Bool | Nil), Bool>:Hash(String, LLVM::Module) +118
[18360849] *Crystal::Compiler#codegen<Crystal::Program, Crystal::ASTNode+, Array(Crystal::Compiler::Source), String>:(Array(String) | Nil) +1313
[18352348] *Crystal::Compiler#compile<Array(Crystal::Compiler::Source), String>:Crystal::Compiler::Result +156
[10148725] *Crystal::Command#spec:NoReturn +3845
[10130417] *Crystal::Command#run:(Array(Crystal::ImplementationTrace) | Array(Crystal::Init::View+:Class) | Array(String) | Bool | Crystal::Compiler::Result | Hash(String, String) | IO::FileDescriptor | Nil) +1425
[10128747] *Crystal::Command::run<Array(String)>:(Array(Crystal::ImplementationTrace) | Array(Crystal::Init::View+:Class) | Array(String) | Bool | Crystal::Compiler::Result | Hash(String, String) | IO::FileDescriptor | Nil) +27
[10128681] *Crystal::Command::run:(Array(Crystal::ImplementationTrace) | Array(Crystal::Init::View+:Class) | Array(String) | Bool | Crystal::Compiler::Result | Hash(String, String) | IO::FileDescriptor | Nil) +25
[9034085] ???
[9092937] main +41
[140509888467601] __libc_start_main +241
[9030586] _start +42
[0] ???

Error: you've found a bug in the Crystal compiler. Please open an issue, including source code that will allow us to reproduce the bug: https://github.com/crystal-lang/crystal/issues

@asterite asterite force-pushed the feature/fix_proc_extern_structs branch from 56bc69d to f160b90 Compare September 15, 2016 18:04
@asterite asterite merged commit a967913 into master Sep 15, 2016
@asterite asterite deleted the feature/fix_proc_extern_structs branch September 15, 2016 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
kind:feature topic:compiler
Projects
None yet
Milestone
0.19.2
Development

Successfully merging this pull request may close these issues.

Lib callbacks work incorrectly when structs are involved
2 participants
@asterite @oprypin