zetzit / zz

πŸΊπŸ™ ZetZ a zymbolic verifier and tranzpiler to bare metal C
MIT License
1.6k stars 52 forks source link

Windows support #15

Closed peirick closed 4 years ago

aep commented 4 years ago

lgtm, but confusing why osx broke. investigating.

aep commented 4 years ago

this is looking great!

if passing the tests is too much work, i would suggest to enable travis in a new PR, or just skip the failing tests, so we at least can get basic windows support in

peirick commented 4 years ago

I got some problems that randomly some moduls like err, string or slice are not found. This seems to be only a problem on windows. Having a look at the logs on travis this seems to be a bug with the loader.rs. Do you have any idea how to fix this ?

aep commented 4 years ago

my best guess would be that something is wrong here https://github.com/aep/zz/blob/master/src/lib.rs#L90

this is the default search paths and in this case these modules are in CARGO_MANIFEST_DIR/modules

also this could be an issue. https://github.com/aep/zz/blob/master/src/project.rs#L126 the load changes directory into the module root maybe changing cwd isnt reliable on windows?

also the hardcoded forward slash here sounds bad https://github.com/aep/zz/blob/master/src/project.rs#L178

peirick commented 4 years ago

Got that bug. was some combination of set_current dir and relative filepaths. set_current dir behaves different on windows than on *nixes.

Next Bug on windows would be about attribute (visibility...) in for example the byteorder module. I my opinion ZZ should not use these attribute extensions but instead should generate c99 compatible Code. As far as I know that means inline functions should be defined in .h files. private functions should be static in .c files. I don't know how to otherwise fix that byteorder module. What do you think?

aep commented 4 years ago

Great work! Thank you so much.

Unfortunately in a family emergency and don't have much attention span right now so just a quick note

Visibility is a big part of exporting a clean abi.

"Pub" functions need to be removed from the abi and they can't be static because other modules need them.

Windows does have an equivalent thing, I remember from Qt.

aep commented 4 years ago

emitter.rs should not emit an attribute but instead some generic macro that is expanded in some sort of prelude.h

That makes it easier to later have it overwritable by the user.

peirick commented 4 years ago

thank you. take your time. familly is important.

that macro would be something like mentioned here: https://stackoverflow.com/questions/19418908/creating-a-portable-library-to-run-on-both-linux-and-windows correct? that prelude.h would be somewhere in the std module?

aep commented 4 years ago

dllexport/import looks correct to me yes.

i'd simply emit the boilerplate at the start of every c file for now. we can make that pretty later. https://github.com/aep/zz/blob/master/src/emitter.rs#L64

also fine for me just disable it on windows for now and refine later

aep commented 4 years ago

amazing! thank you again, that's some great work