Namespace Hack, Part 2
Closed this issue · 12 comments
EXAMPLE PACKAGE 1
Foo::Bar::MyClass
The new NAMESPACE config option solves half the namespace issue, by enabling control of the "Foo::Bar" part of "Foo::Bar::MyClass" above.
The unsolved half of the issue is control over the "MyClass" part of "Foo::Bar::MyClass".
EXAMPLE PACKAGE 2
Foo::Qux::MyClass
We set NAMESPACE to "Foo::Qux" in this case, but the C++ class named "MyClass" is already taken in example 1 above, so we have a C++ conflict. This conflict does not exist in Perl because of the differing "Foo::Bar" and "Foo::Qux" namespaces, but C++ does not have Perl-like namespaces, so the 2 different "MyClass" classes conflict in C++.
POSSIBLE SOLUTION A
Create a new config option named PACKAGE (or CLASS, NAME, etc), in order to complement the new NAMESPACE config option. Like NAMESPACE, the PACKAGE option is a simple string scalar value. If set, the PACKAGE option will be used as the Perl package name to which Inline::CPP binds the compiled C++ class. This solution does NOT work correctly for calls to Inline::CPP where multiple C++ classes are compiled, either from a single C++ file or from a hierarchy of files via C++ "include" directive.
POSSIBLE SOLUTION B
Same as solution A, except PACKAGE may optionally be a hash instead of a string scalar. Each key in the hash is the name of a C++ class, and each corresponding hash value is a string scalar containing the Perl package to which Inline::CPP binds the C++ class. This solution does work correctly for calls to Inline::CPP where multiple C++ classes are compiled, but only where they all fall within the same NAMESPACE, because NAMESPACE is still just a string scalar. This solution does NOT work correctly for non-trivial C++ include file hierarchies.
POSSIBLE SOLUTION C
Allow special setting of NAMESPACE = "AUTO", which will automatically set both NAMESPACE and PACKAGE based on C++ class names. Double underscore "__" in C++ class names is taken to mean double colon "::" scope token in Perl. This leads to a C++ class named "Foo__Bar__MyClass" being parsed to NAMESPACE of "Foo::Bar" and PACKAGE of "MyClass". Likewise, C++ class "Foo__Qux__MyClass" becomes NAMESPACE of "Foo::Qux" and PACKAGE of "MyClass". This solution does work correctly for all cases I can think of at this time, including complex C++ include file hierarchies, such as those generated by RPerl.
It seems to me that the end goal is really solved by supporting C++ namespaces. C++ certainly does have them. Here's what has to happen for us to be able to support C++ namespaces:
1: The grammar needs to detect namespace declarations. (I::CPP::Grammar.pm)
2: The parser needs to maintain state to know what namespace a given class is being compiled into. (I::CPP::Grammar.pm and I::CPP)
3: There needs to be a way for an XS extension to reference a class or function that has been declared within a C++ namespace. (Research - Possibly a patch to Perl's XS system.)
4: The CPP.pm and Inline infrastructure need to support #3.
I would love to support this, but even if I figure out #1 and #2 (which is probably the bigger obstacle), I think there's probably no one individual who knows all the answers to #3. Searching CPAN and asking in XS hasn't turned up any prior art on binding C++ classes and functions that live within C++ namespaces to Perl.
Someone please let me know if they see a way to do this.
Wow I didn't know it would be that complicated to implement real support for C++ namespaces, that makes me sad. This is a critical issue which is blocking significant development for the RPerl compiler.
I can potentially make RPerl work using any one of my proposed solutions A, B, or C above.
I assume it would be easiest to just implement solution A as a workable hack for now?
It seems that 'hiertype' is going to be part of the solution to this. Also as mentioned before, changes to the grammar and parser, including stateful parsing. Still not trivial, and I still haven't found any "proof of concepts" on CPAN. Probably possible. Currently not in progress.
Test t/grammar/18namespaces.t contains some TODO tests that will start passing when/if ever this works out.
This isn't a "squeeky wheel" situation; I am in favor of matching Perl namespaces to C++ namespaces. But it's really only going to be solved when a solution becomes more apparent.
I see the very simplest solution like so, for the purposes of compiling Perl -> C++ which one then wants to have available in Perl.
- Compile Perl to C++, either file or string, mapping Perl classes to C++ with s/::/__/g (possibly with encoding _ as well, some sort of scheme like URL-encoding), eg Class::Name -> Class__Name
- Compile it back into Perl like this:
{ package RPerlCompiled; Inline->bind(CPP => $cpp_code); }
- Call in Perl like so:
RPerlCompiled::Class__Name->method()
I may (or may not) be able to use shim classes or some other mechanism of indirection to work around the current limitations in Inline::CPP. The NAMESPACE config option helps halfway, and I'm not sure that I can create a full work-around solution, but I'll try!
However, I do still respectfully request my proposed solution A be accepted and implemented in the short-term, before further consideration is given to full-blown C++ namespace support. In solution A, all we need is to add a string PACKAGE config option, just like the NAMESPACE option. This would be hugely helpful, and is the simplest proposed solution.
A pull request for this solution, would likely be the best talking point for this issue. Code always speaks louder than words.
I might be able to figure out a way to work around the problem in RPerl, but it won't produce a pull request and it won't help anyone else who uses Inline::CPP. Sorry, I'm not an Inline maintainer and I'm not an XS programmer. :-(
If I'm unable to find a work-around band-aid, then I'll have to come crawling back for help.
From wbraswell via IRC:
I've got an initial test working for the needed "CLASS" config option
https://github.com/wbraswell/Inline-CPP/commit/5ad9bc5020ba179d09a469b3408f01f879390600
the CPP.pm is in a fork I created https://github.com/wbraswell/Inline-CPP/blob/namespace_hack/lib/Inline/CPP.pm
the test driver is in the RPerl repo https://github.com/wbraswell/rperl/blob/master/bin/development/namespace_test.pl
this is an implementation of "solution A" from my github issue #7
I'm also thinking I might also implement solutions B and C
furthermore, mohawk recommended accepting a coderef which takes as input the C++ Class name and as output generates the NAMESPACE and CLASS options
first off, will you please confirm your tentative approval of my implementation of solution A, not counting the copious "warning" commands which I will remove?
(also I will create automated tests instead of just my test driver)
secondly, will you please give your opinion (if any) as to which of the solutions you would like to see implemented?
(personally I would like to use solution C since it gives me the most automated behavior)
My response (irc):
In principle your solution looks good [ed: caveats below].
I would like to see any changes that are not directly related to the solution to be reverted (including comments). It's not that I don't care for your rewordings. Just that I prefer to touch only that code that is part of the patch to minimize the potential for git merge rejections as I'm working on several branches simultaneously. I would also need to see tests in t/class_override/*.t that are approximately similar in spirit to those in t/namespace/*.t
And finally, we would want to see documentation supporting the keyword. Again, touching only the portion of the POD that deals specifically with this class keyword (this is important because I'm also working within the POD at the moment.) At that point I should be able to respond favorably to a pull request.
[ed: Backpedaling now as an issue has been spotted with the proposal.]
hold... thinking for a moment...
It's not complete if it doesn't allow for a mapping of c++ class => new name, other c++ class => other new name, etc. That wouldn't be hard to implement as a hash.
So use Inline CPP => config => class => { cpp_name1 => 'perl_name', cpp_name2 => perl_name2 };, etc.
So there's the bullet list; accept a hashref that maps cppname to perlname for multiple classes, write tests to prove it works, revert changes that touch irrelevant parts of code, document the feature in one place using an example and a few lines of text, issue pull request :)
wbraswell's pull request has been adapted and merged into master, and released as CPAN v0.57.