two fixes for XS INTERFACE keyword #23640
Open
+130
−30
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Sounds exciting. I may want to try them out, I know exactly which project could use this. |
XS currently allows a Perl package name to be used where a C type is
expected; behind the scenes, s/:/_/g is performed on the type string.
For example this XS declaration,
X::Y
foo(...)
causes this C declaration to be emitted for RETVAL:
X__Y RETVAL;
However, in the presence of the INTERFACE keyword, e.g.
X::Y
foo(...)
INTERFACE: ....
an extra variable to hold a function pointer is declared and
initialised, but the return type of the function gets declared as X::Y
rather than X__Y, resulting in uncompilable C code being generated.
This commit fixes that.
Before, the code emitted was:
dXSFUNCTION(X::Y);
XSFUNCTION = XSINTERFACE_FUNC(X::Y,cv,XSANY.any_dptr);
but is now:
dXSFUNCTION(X__Y);
XSFUNCTION = XSINTERFACE_FUNC(X__Y,cv,XSANY.any_dptr);
GH #23192 The INTERFACE keyword in XS allows the same XSUB to wrap multiple C library functions, by storing a function pointer to the C function in each CV. This has started failing on some strict C compilers, as the C code generated by the XS compiler does some suspect function pointer casts. This commit fixes the issue by providing a more correct cast in most cases (it still can't handle non-trivial C_ARGS values). Background: Before this commit, the presence of the INTERFACE keyword in this XSUB: char* foo(int a, int b, int c) INTERFACE: bar baz would cause these extra lines to be added to the XSUB's body: dXSFUNCTION(char*); XSFUNCTION = XSINTERFACE_FUNC(char*,cv,XSANY.any_dptr); RETVAL = XSFUNCTION(a, b, c); and lines like this to be added to the boot XSUB: XSINTERFACE_FUNC_SET(cv,bar); After macro expansion, these lines look roughly like the following: char* (*XSFUNCTION)(); XSFUNCTION = (char* (*)()))(XSANY.any_dptr)); RETVAL = XSFUNCTION(a, b, c); CvXSUBANY(cv).any_dxptr = (void (*) (pTHX_ void*))(bar); The issue: The specific error the C compiler is giving in the ticket is: error: too many arguments to function ‘XSFUNCTION’ which is caused by this line: RETVAL = XSFUNCTION(a, b, c); because XSFUNCTION has been declared as a pointer to a function which has no args, and is now being used as a pointer to a function which takes args. The fix: This commit fixes this issue by adding adding a cast: but only to the place where XSFUNCTION is used to call the function. The other places are left untouched (so the XSFUNCTION variable itself still has the "wrong" type). This has the advantage that the various dXSFUNCTION etc macros in XSUB.h don't need to be modified, and so portability to older Perls is less of an issue. It also means that INTERFACE_MACRO usage should be unaffected. After this commit, the code emitted to call XSFUNCTION now looks like: RETVAL = ((char* (*)(int,int,int))(XSFUNCTION))(a, b, c); This cast is generated based on the types of the XSUB's parameters and return values. Things are difficult in the presence of C_ARGS; for example, char* foo(int a, int b, int c) INTERFACE: bar baz C_ARGS: a, c In this case, the XS parser now splits the C_ARGS line and looks up the type of each corresponding parameter. If it can't be found, it uses 'void *' instead and hopes for the best. For more complex C_ARGS entries such as C_ARGS: foo(a), b+1 this doesn't work and it is likely that uncompilable C code will still be generated, This commit adds tests both to 001-basic.t (which just checks that the generated C code looks as expected), but also to 002-more.t, which actually compiles and executes the C code, and thus is more likely to catch failures due to picky C compilers.
iabyn
force-pushed
the
davem/xs_interface
branch
from
9bea2f9 to
d73b81a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A few commits to fix a couple of issues with the INTERFACE keyword in XS.
First, INTERFACE didn't support Perl package names as C types (i.e. Foo::Bar not getting auto-converted to Foo__Bar).
Secondly, XS code using INTERFACE was starting to fail on picky C compilers due to type-casting issues (GH #23192).