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

Support XS written in C++ #195

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mohawk2
Copy link
Member

@mohawk2 mohawk2 commented Jan 7, 2015

Files ending in .xscc will be transparently converted to .cpp and then built. Doc recommends use of ExtUtils::CppGuess for compiler config info - I anticipate having to work on that module to increase its applicability, which I am content to do. t/04-xscpp.t will skip_all unless ExtUtils::CppGuess is installed.

@vovkasm, @daoswald, @rehsack, your thoughts on this extremely welcome.

@daoswald
Copy link

daoswald commented Jan 7, 2015

I like the premise. It seems like an unambiguous way to deal with C++ extensions.

@mohawk2
Copy link
Member Author

mohawk2 commented Jan 7, 2015

In order to support simultaneous C and C++, there may well be different values of (eg) CC for each. This could be supported using the existing XSBUILD option:

XSBUILD => {
  xs => {
    all => \%cbuild_opts,
    'lib/Foo/Other' => { %cbuild_opts, 'DEFINE' => "$defines -Dspecific" },
  },
  xscc => { all => \%cppbuild_opts },
  xsf => { all => \%fortranbuild_opts },
}

which is already used in xs_o to pick how to build each extension.

@vovkasm
Copy link
Contributor

vovkasm commented Jan 7, 2015

Hi! I think this idea even better that in #45. Because defined interface (file suffixes) allows more improvements without change anything in user code.

@Leont
Copy link
Member

Leont commented Jan 9, 2015

I do not see why we want code that is this unportable in the core toolchain. All of this can be achieved in an extension too. And if it really couldn't, that should be fixed.

Also, it fails on my system, didn't look into why yet.

@mohawk2
Copy link
Member Author

mohawk2 commented Jan 9, 2015

If you could put the error text on an EUCG issue that would be very helpful.

@mohawk2
Copy link
Member Author

mohawk2 commented Jan 12, 2015

@vovkasm, if this PR solves the C++ problem well, do you feel like #45 is therefore obsolete and could be closed?

@vovkasm vovkasm mentioned this pull request Jan 14, 2015
@mohawk2 mohawk2 force-pushed the xscplusplus branch 2 times, most recently from cf87edc to af7abc3 Compare January 19, 2015 22:33
@Leont
Copy link
Member

Leont commented Jan 28, 2015

This pull request solves a non-existent issue (triggering XS's C++ mode), but doesn't solve the hard problem (how to use/configure a C++ compiler). IMNSHO it's complete bogus.

@rehsack
Copy link
Contributor

rehsack commented Jan 28, 2015

From my point of view it is doing the second step before the first. It provides a seemingly working solution which is far away from being consistent or provable.

But releasing such solutions to wildlife results in people start relying on it - which makes it incredible difficult to fix root causes of mistakes later.

Generally, any extension of core toolchain should be added as plugin, if possible.

@mohawk2
Copy link
Member Author

mohawk2 commented Jan 28, 2015

The second step (of correctly detecting how to compile C++) is indeed a hard one, but work is in progress to make EU::CppGuess even more widely useful.

In the meantime, the EUMM change proposed is simply to enable the EUMM side of things to be easy. It doesn't even need a plugin.

@Leont
Copy link
Member

Leont commented Jan 28, 2015

In the meantime, the EUMM change proposed is simply to enable the EUMM side of things to be easy. It doesn't even need a plugin.

It doesn't. EU::CppGuess doesn't need any of these changes. What use-case is made simpler by this PR?

@mohawk2
Copy link
Member Author

mohawk2 commented Jan 28, 2015

Leon, I'm sorry but I don't understand. EUCG here is a producer of C++ config info, not a consumer of EUMM needs.

The PR enables much simpler XS extensions with C++, and the feedback I have had from makers of such is unanimously positive towards this.

@Leont
Copy link
Member

Leont commented Jan 28, 2015

The PR enables much simpler XS extensions with C++

Does it? What problem is solved by this that isn't solved by «XSOPT => "-C++"?

and the feedback I have had from makers of such is unanimously positive towards this.

That doesn't mean complicating our codebase is the right solution.

@mohawk2 mohawk2 force-pushed the xscplusplus branch 2 times, most recently from 28a2f1c to 3d844b7 Compare May 8, 2016 13:41
@mohawk2
Copy link
Member Author

mohawk2 commented May 9, 2016

Having just rebased and re-looked at this, I have re-thought it.

First of all, this is not worth putting effort into until EUMM is nice and stable again.

I still think there is a problem of EUMM not supporting XS of things other than C, but it ought to be hidden away behind XSBUILD options entirely, rather than adding (much) to the global EUMM code. The good news is that ought to be (when I look at this properly) rather easy, and putting it behind XSBUILD ought to drastically reduce the risk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants