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.

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

Reuse namespaces #100

Open
gchenfc opened this issue Apr 22, 2021 · 6 comments
Open

Reuse namespaces #100

gchenfc opened this issue Apr 22, 2021 · 6 comments
Labels
feature New feature or request help wanted Extra attention is needed

Comments

@gchenfc
Copy link
Member

gchenfc commented Apr 22, 2021

Currently, I believe the wrapper will break if you open, close, then reopen a namespace. For example:

namespace ns {
  int a = 5;
}

namespace ns {
  int b = 6;
}

Since the wrapper is not smart enough to realize that these are the same and hence it doesn't have to redefine the submodule pybind11::module m_ns = ....

Aside from minor inconveniences of writing a .i file to remember to put all the stuff from a namespace at the same place, this causes issues using combine_interface_headers which just copy-pastes the contents together. Thus if you want to "add" to a namespace from a different interface header, it won't work.

Not sure whether or not the goal of having a nice pybind way to #include other .i files would be able to handle this, or if we just need some intelligent way of keeping track which namespaces have been handled. For example, a 2-pass approach: first go through all the namespaces and just define them and keep a set() of defined namespaces, then second pass do all the "real" wrapping.

@gchenfc gchenfc added bug Something isn't working feature New feature or request help wanted Extra attention is needed labels Apr 22, 2021
@ProfFan
Copy link
Collaborator

ProfFan commented Apr 22, 2021

We are going one step at a time towards a real C++ compiler in Python... 🤣

@gchenfc
Copy link
Member Author

gchenfc commented Apr 22, 2021

lol ikr 😂

@varunagrawal
Copy link
Collaborator

*transpiler

@varunagrawal
Copy link
Collaborator

Jokes aside, this is solvable via a 2 pass compiler rather than the current 1 pass compiler scheme. Though this would have come up when fixing #44 which I believe is what we should focus on.

@ProfFan
Copy link
Collaborator

ProfFan commented Apr 24, 2021

It's high time that we have roundtrip tests from header to AST to header 🤣

@varunagrawal
Copy link
Collaborator

I potentially came up with a solution to this in #118. Basically, use a dict to keep track of the namespace name and then just add it to the content. I have spent way too much time on multi-file support, so I'll leave this to someone else. 🙂

@varunagrawal varunagrawal removed the bug Something isn't working label Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants