Split infrastructure types into zngur.h#67
Split infrastructure types into zngur.h#67nickpdemarco wants to merge 8 commits intoHKalbasi:mainfrom
Conversation
|
@HKalbasi looks like rust-lang/rust#115746 is hitting us again, that panic behavior is now in stable. How would you like me to handle this? |
|
Thanks for the PR! I'm generally on board with #48 and this implementation looks good for the start. Is it feasible to also keep the single file version working and make it configurable? I think the single file version is more suitable for small projects and people who want to try zngur. It also addresses the backward compatibility issue. Backward compatibility is not a dead end, but it is good to avoid breakage if possible, specially for this kind of breakage which affect every project.
Ah, that was decided to keep the thread id. But we can do the workaround proposed there (normalizing the output and removing the thread id) and keep the ability of auto-fixing the tests. |
|
I implemented the normalizing so CI should become green if you rebase on the main branch. |
|
@HKalbasi I think it should be possible to support the old behavior, too. My preference is to have the new split mode the default. Would you object to smaller projects needing to invoke |
|
I don't have a strong opinion on the default. We can even go far and have no default, force user to provide either |
|
@nickpdemarco Do you mind if I try to get this over the finish line? |
|
@adsnaider please do! Thank you. Happy to review any changes. |
|
@nickpdemarco, I have a POC but it's running into some issues when it comes to namespaces because of how they are currently handled. Essentially, the zngur-parser and generator don't know where symbols would be located when generating the C++ output. Aside from that, it works on all other examples that make no use of Alternatively, we need to disable namespaces altogether which seems bad but also it may be okay given that we already use the Rust module for namespacing |
|
See #104 for different implementation |
Working towards #48, this PR splits zngur infrastructure types (
rust::Unit,rust::Ref,rust::RefMut, and others) into a newzngur.hthat is#included bygenerated.h.This obviously comes with some infrastructure changes. Now, clients must be concerned with include paths - notice the changes to the example
Makefiles to now add-I.to their C++ compiler invocations. We also now have a-o path/to/output/dirargument to zngur.I think this is a reasonable milestone to get some feedback on the overall design. Please let me know what you think! If we do merge this, I think it should be a breaking change. We could make the default argument to
-obe the CWD to approximate backwards compatibility, but I think that runs the risk of confusing users.