MrSmith33 / vox

Vox language compiler. AOT / JIT / Linker. Zero dependencies
Boost Software License 1.0
338 stars 18 forks source link

Add extern module attribute #15

Closed jedekar closed 3 years ago

jedekar commented 3 years ago

A few points I want to comment on:

MrSmith33 commented 3 years ago

A few points I want to comment on:

  • This copy-and-paste test tests only parsing, but I didn't see any attribute check for syscall case.

Yea, the way it works right now would require generating an executable for linux, which I didn't do. Ideally needs a test too.

  • Doesn't the addition of module name to identifiers corrupt idMap? For example, I want a function/variable kernel32, won't they conflict with each other?

No, because idMap is just used to intern strings for O(1) comparison on identifiers. Storing strings there is fine.

  • I changed the type of data to Identifier, and now syscall case uses index field as a syscall number, which is bad. I just think I misunderstood you somewhere. I thought about the union of Identifier and uint for data, but that's bad too. Isn't data supposed to be generic and low-level, like []u8 or something. It means we can place there anything, we just need a way for interpreting it.

Commented on that.

Forgot to mention, that you need to strip " on string literal, before putting it into idMap. Also, check the test output of driver.context.printAstSema = true; in tester. Did you figure out how to run single test?

jedekar commented 3 years ago

Yeah, sure, I read the build/testing manual. I didn't get to see the output for that exact test, because I corrected it. It brought another issue, though.