inline support

[plicease]

I am not 100% sure this is what was intended by #71, but it does make Inline / Alien integration easy, as can be seen by the .t file included in this patch.

[plicease]

TODO (if agreed on):

1. add an example to synopsis
2. install Inline::C in .travis.yml so this gets tested on travis. (right now travis seems dead)

[mohawk2]

Great work! This is 90% of the way there - I'd like to see libdontpanic.h be in the ->Inline's https://metacpan.org/pod/Inline::C#auto_include , and that the test actually calls a function in the lib rather than one supplied here, probably using https://metacpan.org/pod/Inline::C#autowrap .

[plicease]

I think that might be tricky. How does Alien::Base know which .h files to include. In a lot of cases simply scanning for all .h files will not be appropriate.

edit having trouble with Boolean logic today.

[mohawk2]

I would prefer not to automatically include them all. Instead, what do we think of a ->auto_include method that in AB returns empty string, and can be over-ridden?

[plicease]

I think the default really should be not to automatically include anything.

edit for clarity

[plicease]

I don't mind an alien_ option in AB::MB to force the behavior that you are talking about though.

[mohawk2]

The default certainly should be to not include anything. I'm seeking discussion on the technique for changing the default:

• overridable auto_include method in subclass?
• Have subclass simply override Inline method (probably least elegant/reusable)?
• Data that the superclass Inline accesses (set with one of these alien_ options)?

[plicease]

Ah I see what you are saying.

Let me play with it some more. I think I like the auto_include method.

[plicease]

Tried adding a C++ test for this as well, but it is dumping when it calls answer().

diff --git a/lib/Alien/Base.pm b/lib/Alien/Base.pm
index 220b999..6f1191f 100644
--- a/lib/Alien/Base.pm
+++ b/lib/Alien/Base.pm
@@ -369,7 +369,7 @@ sub dynamic_libs {

 sub Inline {
   my ($class, $language) = @_;
-  return if $language ne 'C';
+  return if $language !~ /^(C|CPP)$/;
   return {
     CCFLAGSEX => $class->cflags,
     LIBS      => $class->libs,

use strict;
use warnings;
use Test::More;

BEGIN {
  eval { require Inline; require Inline::CPP; } || plan skip_all => 'test requires Inline and Inline::CPP';
  eval { require Acme::Alien::DontPanic; } || plan skip_all => 'test requires Acme::Alien::DontPanic';
}

use Acme::Alien::DontPanic;
use Inline with => 'Acme::Alien::DontPanic';
use Inline CPP => 'DATA';

is Foo->new->string_answer, "the answer to life the universe and everything is 42";

done_testing;

__DATA__
__CPP__

#include <libdontpanic.h>
#include <stdio.h>

class Foo {
public:
  char *string_answer()
  {
    static char buffer[1024];
    sprintf(buffer, "the answer to life the universe and everything is %d", answer());
    return buffer;
  }
};

changing answer() to 42 makes this test pass, so it is definitely dumping core on calling answer(). I was able to call answer() from a simple .cpp file once I updated libdontpanic.h to support C++.

If anyone can see what I am doing wrong please let me know.

[plicease]

Added autowrap with the test.

as for auto include, I think I sort of changed my mind. I am thinking more along the lines of the third option:

Data that the superclass Inline accesses (set with one of these alien_ options?

Just because, you can do most things from the Build.PL and just have a mostly empty Alien::Base subclass. Possibly implement this in Alien::Base with the first option

overridable auto_include method in subclass?

For those that want control at XS build time, as apposed to Alien build time.

I will sleep on it. If anyone has any thoughts on this, let me know.

[daoswald]

I haven't looked at the failure yet, but I would like to suggest that the regular expression, "return if $language !~ /^(C|CPP)$/;" is inadequate for Inline::CPP. If you check the source for Inline::CPP (CPP.pm) you'll see that we also support the following aliases: 'cpp', 'C++', 'c++', 'Cplusplus', 'cplusplus', 'CXX', and 'cxx'.

I notice this is undocumented, and probably should just be deprecated and removed eventually. But for now, it would would be wise for the regex to look like this:

return if $language !~ /^(C|CPP|cpp|CXX|cxx|Cplusplus|cplusplus|C++|c++)$/;

Case-insensitivity wouldn't be a good approach because some case-variations are not covered by the existing aliases.

[plicease]

Seems like the language passed into Inline gets normalized somewhere, because using CXX works for this test:

twin% git diff
diff --git a/t/inline_cpp.t b/t/inline_cpp.t
index 06b5da9..60b0a5a 100644
--- a/t/inline_cpp.t
+++ b/t/inline_cpp.t
@@ -9,7 +9,7 @@ BEGIN {

 use Acme::Alien::DontPanic;
 use Inline with => 'Acme::Alien::DontPanic';
-use Inline CPP => 'DATA', ENABLE => 'AUTOWRAP';
+use Inline CXX => 'DATA', ENABLE => 'AUTOWRAP';

 SKIP: {
   skip "broken", 1;
@@ -20,7 +20,7 @@ is answer(), 42, "direct";
 done_testing;

 __DATA__
-__CPP__
+__CXX__

 #include <libdontpanic.h>
 #include <stdio.h>
twin% prove -lv t/inline_cpp.t
t/inline_cpp.t ..
ok 1 # skip broken
ok 2 - direct
1..2
ok
All tests successful.
Files=1, Tests=2,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.10 cusr  0.00 csys =  0.12 CPU)
Result: PASS

[plicease]

...and of course it doesn't dump core with a debugging perl...

twin% pb use perl-5.18.2g3@dev
twin% prove -lv t/inline*.t
t/inline.t ......
ok 1 - indirect call
ok 2 - direct call
1..2
ok
t/inline_cpp.t ..
ok 1 - indirect
ok 2 - direct
1..2
ok
All tests successful.
Files=2, Tests=4,  0 wallclock secs ( 0.05 usr  0.01 sys +  0.44 cusr  0.02 csys =  0.52 CPU)
Result: PASS

[mohawk2]

@plicease, putting aside the Heisenbug nature of the debugging perl issue - what steps would @daoswald take to reproduce the Inline::CPP -> core dump, from scratch?

[plicease]

1. Install Acme::Alien::DontPanic
2. commencement out the skip statement from t/inline_cpp.t
3. run prove -lv t/inline_cpp.t or perl -Ilib t/inline_cpp.t from this branch.

For me it consistently fails, on non-debugging Perls. I've only tested it so far on Linux.

I did this
https://github.com/plicease/Alien-Base-Extras/commit/bb125adcd4c35a9478af543575f4de537a16cf90
hoping it might help but it did not.

I will take another look this week when I get a chance too. To at least figure out if it is something with my environment or not.

[plicease]

Fixed the travis build in this branch. I am still getting a failure with 5.8 and ALIEN_FORCE=0

https://travis-ci.org/plicease/Alien-Base/jobs/35895848

pretty sure this must be a AB rather than Inline issue though.

[plicease]

nevermind, this looks as though it is in fact due to the C++ unfriendly header file as I originally suspected. I can get the dump consistently by declaring answer() as a regular c++ extern. I think when I thought I was running with the corrected header file I must have been using a system version which was not corrected, or perhaps I had forgotten to clear an _Inline cache. Either way, please consider this to be user error on my part.

twin% git diff
diff --git a/t/inline_cpp.t b/t/inline_cpp.t
index 4f68f5b..0e83fd8 100644
--- a/t/inline_cpp.t
+++ b/t/inline_cpp.t
@@ -21,9 +21,11 @@ done_testing;
 __DATA__
 __CPP__

-#include <libdontpanic.h>
+/* #include <libdontpanic.h> */
 #include <stdio.h>

+extern int answer();
+
 class Foo {
 public:
   char *string_answer()

[plicease]

I prefer #78 to this PR, but failing consensus on that I would vote aye for this PR.

[mohawk2]

@plicease, I vote "aye" on whichever you think is better.

[plicease]

Closing as it seems agreed preference is on #78