Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl: the Markov chain saw
 
PerlMonks  

ExtUtils::ParseXS has changed

by syphilis (Archbishop)
on Aug 17, 2011 at 06:31 UTC ( [id://920621]=perlquestion: print w/replies, xml ) Need Help??

syphilis has asked for the wisdom of the Perl Monks concerning the following question:

Hi,
The PDL source has some XS code that looks like basically this:
SV * at_c(x,position) pdl* x PDL_Long * pos = NO_INIT CODE: // code that does stuff OUTPUT: RETVAL
And that format has been working quite well for quite some time - and still works with ExtUtils-ParseXS-2.2210.

But, with the latest ExtUtils-ParseXS-3.03 that produces the error "Could not find a typemap for C type 'PDL_Long *'".
It seems that having the PDL_Long * pos declaration *before* the CODE block is now sending the message that the PDL_Long * type needs to be bound to perl via a typemap entry. (Perhaps it should *always* have been sending that message ? ... I don't know.)

I can fix the problem by altering the code to:
SV * at_c(x,position) pdl* x CODE: PDL_Long * pos; // code that does stuff OUTPUT: RETVAL
Is that the right thing to do ?
Or is ExtUtils-ParseXS-3.03 doing something it shouldn't ?

Btw, I don't know what NO_INIT is. It doesn't seem to be defined in the PDL code, so I guess it's some XS symbol.
I should also mention that PDL_Long is just a typedef for int (in pdl.h).

Cheers,
Rob

Replies are listed 'Best First'.
Re: ExtUtils::ParseXS has changed
by Anonymous Monk on Aug 17, 2011 at 06:53 UTC

    What file?

    IIRC, I don't think it likes // comments , I think it likes /* */ comments, at least that is my recollection of my toying with this

    http://perldoc.perl.org/perlxs.html#The-NO_INIT-Keyword

    I'm just finished compiling PDL-2.4.9_005 with ExtUtils::ParseXS: 3.03 as I type this

    No problems so far -- I guess I remembered correctly, must be the comments, use /**/

      What file?

      Basic/Core/Core.xs.PL

      I'm just finished compiling PDL-2.4.9_005 with ExtUtils::ParseXS: 3.03

      Yes - it should build fine, but only because its Basic/Core/typemap has a pointless typemap entry for 'PDL_Long *'.
      Well ... of course, it's *not* pointless .... it prevents xsubpp from complaining about the lack of a typemap entry. But the typemap has no INPUT or OUTPUT entry for 'PDL_Long *' - it therefore does nothing other than trick xsubpp into believing that the typemapping has been done, thereby silencing the complaint.
      Of course, no such typemapping has been done and I don't think this is the right solution.

      I guess I remembered correctly, must be the comments, use /**/

      If you say so ....

      Cheers,
      Rob

      UPDATE: I suppose I should take the time to point out that the function I gave by way of example doesn't contain literally // code that does stuff
      That just stands for a block of code whose actual content is irrelevant to the question I've asked.
      Anonymous Monk, if you want to see the failure, try building 2.4.9_004 with ExtUtils-ParseXS-3.03. The error should occur about one minute (if that) into the 'make' stage.

        ...has a pointless typemap entry for 'PDL_Long *'... has no INPUT or OUTPUT ...

        I don't think there is any tricking going on

        The default typemap has

        T_PTR $var = INT2PTR($type,SvIV($arg))

        I don't think it needs INPUT/OUTPUT

        Since PDL_Long * pos is the RETVAL, and the return type is SV*, you need a typemap entry

        You might not need it if you specified T_PTR as return type

        T_PTR at_c(x,position) pdl* x PDL_Long * pos = NO_INIT CODE: // code that does stuff OUTPUT: RETVAL

        I would test it, but, PDL takes a while to compile :)

Re: ExtUtils::ParseXS has changed (PREINIT:)
by tye (Sage) on Aug 17, 2011 at 18:33 UTC

    Well, you certainly should not put non-arguments there. The line after "pdl* x" should declare the type of the "position" argument. It should not be some random bit of code about "pos", which is not one of the arguments.

    The fact that it used to work is certainly an accident.

    But the correct thing to do is not what you proposed but more like:

    SV * at_c(x,position) pdl* x SV* position PREINIT: PDL_Long * pos; CODE: ... OUTPUT: RETVAL

    Then you can also replace "ST(1)" with "position" in the code.

    "The PREINIT: keyword allows extra variables to be declared immediately before or after the declarations of the parameters from the INPUT: section are emitted."

    - tye        

      The fact that it used to work is certainly an accident

      Ok - that makes good sense to me, and is one of the things I was hoping to establish.

      Then you can also replace "ST(1)" with "position" in the code.

      Some of us were wondering what this unused "position" argument was doing. Obviously it wasn't doing anything, but I think I start to understand a purpose for it. (Update: Duh !! It's just an arg that can now be accessed by its name instead of its position on the stack.)

      As I now understand it, it's *all* of the other variables (not just 'pos') that should be declared in PREINIT instead of CODE.

      I'll rewrite the file in accordance with tye's advice and give it a run.
      Many thanks to all respondents.

      Cheers,
      Rob
        Being the guy who did the changes to ExtUtils::ParseXS, let me add my support for what tye said: The code in question worked purely by accident. The proper way to declare or initialize variables in an XSUB is in a PRE_INIT block! ExtUtils::ParseXS 3.XX just became strict in telling you about the problem in your code. It could have instead broken your code to segfault because of the old behaviour being an accidental misfeature and side-effect of the horrors that lurk in EU::ParseXS' code generation.

        As I now understand it, it's *all* of the other variables (not just 'pos') that should be declared in PREINIT instead of CODE.

        C doesn't allow intermixing of vars declarations and code like Perl and C++. Var declarations can just be placed globally or at the start of the block. PREINIT will be placed before any code. CODE can be preceded by other code. Therefore, declarations must be placed in PREINIT or you must add curlies to CODE.

        The PREINIT section of perlxs explains this in more detail and gives example.

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://920621]
Approved by ww
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others meditating upon the Monastery: (5)
As of 2024-04-19 12:37 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found