-
Notifications
You must be signed in to change notification settings - Fork 340
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
Improvements to namespace support #353
Comments
The second and third items on the list are probably #166. I'm going to need to get at least half way down this list before succeeding in google/autocxx#50. |
@adetaylor are you already working on one or more of these? If not I'm going to start working through them again. |
Yes. I have started with the second item. I'll aim to post my current progress somewhere by the end of today, assuming I can eke out any time at all (today seems very distraction-filled) |
OK, here's my current progress on item 2 of the list. Depending on work and family commitments I may have a crack at 1, 3 and 4 tomorrow (I hope I can pinch some of #298 for the third item on the list). Meanwhile @dtolnay if you have a chance to tell me if I'm along the right lines that'd be great! |
Ok, I'd prefer if you let me know which ones you're definitely going to take on and which you aren't as you know, so that I can take them on otherwise. |
I have pretty much done 2 and then 1. Both are now pushed to the branch mentioned just above. Would you like to branch that branch and start to work on 3 and 4 (and most importantly, some tests for 3 and 4)? All the work on those should be in (The last thing I'm going to be doing here is trying to remove some duplication between |
More progress here from a bit of tinkering this evening (a mess of commits which I will gradually rebase and clean up). @sbrocket please post here if you're likely to start any part of this, so we don't collide. 90% of this is tests (which of course fail horribly). The remaining 10% is that I took some of the attribute parsing code from your previous work in #298 and have plumbed it in per items 3 and 4 on the list at the top. I ended up requiring the syntax adetaylor/cxx@always-emit-namespaces...adetaylor:allow-namespace-override Next steps:
|
I was planning on working on 3 and 4 based on your previous message but not over the weekend. If you want to just finish it that’s fine with me, I’ll check where you are on Monday. |
OK great. |
Hm. I think I'm going to have to take a slightly different approach here anyhow. The various maps inside |
I've closed #369 for now as I think it needs some rework. When we encounter a type declaration/definition, we know its C++ namespace and can fully qualify its name (that's what that PR did). However, when we encounter a type reference e.g. inside a struct, we don't know its C++ namespace. We were previously invalidly qualifying it with the currently applicable namespace. We therefore need to do this:
This requires more changes to the various data structures in |
OK. I got the tests to pass (bearing in mind I have only added tests for types in various namespaces, not yet functions). At the moment the branch referenced above contains many muddled commits and duplication which needs a lot of clearing up, so don't take a look yet! |
Progress update: I've added all the tests that I said I would, and they all pass. I'm now gradually rebasing and squashing the branch into a coherent series of commits. To reiterate, this will cover points 1-4 of the plan at the top here. |
OK, if you've got 1-4 covered then I'll wait to review the PR for those, since I think that should cover everything I need for my use. I'll confirm that after you post a PR and see if there's any gaps still, and then afterwards I can loop back and consider doing 6 and/or 7 since those aren't required (for me, anyway). |
@dtolnay Would you like help with the last item in the list? I'd like to take a crack at it if no one else is working on it to get this issue closed out. |
The last item is required for codebases that use both |
Requires not-yet-developed `use` statements in `cxx::bridge` blocks: dtolnay/cxx#353
This issue is just to track the implementation plan laid out at the bottom of #298 (comment).
Emit all mentions of a type in generated C++ code (in function arguments and so forth) as a fully qualified pathAllow namespace override #370::name::space::of::Type
, even if it currently always happens to be within the same module.Store a distinct Namespace on each FFI item rather than having a single global namespace for the whole code generation.Allow namespace override #370ImplementAllow namespace override #370 Switch to #[namespace = A::B] syntax. #380#[namespace = ...]
attribute to determine namespace of individual items.Implement hierarchical handling of namespace which prioritizes item-level attribute > extern block > module.Parse namespace attribute on foreign mods #444Emit ExternType impls for shared structs.Emit an ExternType impl for shared structs and enums #341Safe ExternType derive for extern Rust items.16e2620extern "Rust" { #[derive(ExternType)] type TheType; }
Permit
use
inside of cxx::bridge.The text was updated successfully, but these errors were encountered: