Mailing List Archive: Patch for XSUB.h

Patch for XSUB.h

Tim.Bunce at ig

Aug 25, 1995, 5:59 AM

Post #1 of 9 (2596 views)

This patch removes spurious semicolons, makes returning yes/no/undef
more efficient, adds an XSRETURN_EMPTY and a comment reminding people
to use braces around the XSRETURN_* macros.

(Guess what bug I just found in my code :-)

*** ./XSUB.h.1m Wed Aug 16 18:45:11 1995
--- ./XSUB.h Fri Aug 25 13:50:42 1995
***************
*** 20,30 ****
/* Simple macros to put new mortal values onto the stack. */
/* Typically used to return values from XS functions. */
! #define XST_mIV(i,v) ST(i)=sv_2mortal(newSViv(v));
! #define XST_mNV(i,v) ST(i)=sv_2mortal(newSVnv(v));
! #define XST_mPV(i,v) ST(i)=sv_2mortal(newSVpv(v,0));
! #define XST_mNO(i) ST(i)=sv_mortalcopy(&sv_no);
! #define XST_mYES(i) ST(i)=sv_mortalcopy(&sv_yes);
! #define XST_mUNDEF(i) ST(i)=sv_newmortal();

#define XSRETURN_IV(v) XST_mIV(0,v); XSRETURN(1)
#define XSRETURN_NV(v) XST_mNV(0,v); XSRETURN(1)
--- 20,32 ----
/* Simple macros to put new mortal values onto the stack. */
/* Typically used to return values from XS functions. */
! #define XST_mIV(i,v) ST(i)=sv_2mortal(newSViv(v))
! #define XST_mNV(i,v) ST(i)=sv_2mortal(newSVnv(v))
! #define XST_mPV(i,v) ST(i)=sv_2mortal(newSVpv(v,0))
! #define XST_mNO(i) ST(i)=&sv_no
! #define XST_mYES(i) ST(i)=&sv_yes
! #define XST_mUNDEF(i) ST(i)=&sv_undef

+ /* Note that these are not single statements. Remember to use */
+ /* braces around them if needed. Don't say: if (foo) XSRETURN.. */
#define XSRETURN_IV(v) XST_mIV(0,v); XSRETURN(1)
#define XSRETURN_NV(v) XST_mNV(0,v); XSRETURN(1)
***************
*** 33,34 ****
--- 35,37 ----
#define XSRETURN_YES XST_mYES(0); XSRETURN(1)
#define XSRETURN_UNDEF XST_mUNDEF(0); XSRETURN(1)
+ #define XSRETURN_EMPTY XSRETURN(0)

Tim.

p.s. Larry, this patch supersedes the earlier XSUB.h patch I sent you
which just had the yes/no/undef changes.

Re: Patch for XSUB.h [ In reply to ]

Aug 25, 1995, 9:15 AM

Post #2 of 9 (2583 views)

At 01:59 PM 8/25/95 +0100, you wrote:
... clip ... clip ...

>+ /* Note that these are not single statements. Remember to use */
>+ /* braces around them if needed. Don't say: if (foo) XSRETURN.. */
> #define XSRETURN_IV(v) XST_mIV(0,v); XSRETURN(1)
> #define XSRETURN_NV(v) XST_mNV(0,v); XSRETURN(1)
>

... clip ... clip ...

Tim,

Why not add the braces to the macros?
eg:
#define XSRETURN_IV(v) { XST_mIV(0,v); XSRETURN(1); }

---- Doug

---------------------------------------------------------------------
Don't be mislead by facts.

Re: Patch for XSUB.h [ In reply to ]

Aug 25, 1995, 9:42 AM

Post #3 of 9 (2575 views)

According to Tim Bunce:
> This patch removes spurious semicolons, makes returning yes/no/undef
> more efficient, adds an XSRETURN_EMPTY and a comment reminding people
> to use braces around the XSRETURN_* macros.

Two suggestions:

1. Parenthesize the asssignment macros:

#define XST_mIV(i,v) (ST(i)=sv_2mortal(newSViv(v)))

2. Instead of commenting on the need for braces, fix the bug:

Index: XSUB.h
***************
*** 27,34 ****
#define XST_mUNDEF(i) ST(i)=sv_newmortal();

! #define XSRETURN_IV(v) XST_mIV(0,v); XSRETURN(1)
! #define XSRETURN_NV(v) XST_mNV(0,v); XSRETURN(1)
! #define XSRETURN_PV(v) XST_mPV(0,v); XSRETURN(1)
! #define XSRETURN_NO XST_mNO(0); XSRETURN(1)
! #define XSRETURN_YES XST_mYES(0); XSRETURN(1)
! #define XSRETURN_UNDEF XST_mUNDEF(0); XSRETURN(1)
--- 27,34 ----
#define XST_mUNDEF(i) ST(i)=sv_newmortal();

! #define XSRETURN_IV(v) if (0) {} else { XST_mIV(0,v); XSRETURN(1); }
! #define XSRETURN_NV(v) if (0) {} else { XST_mNV(0,v); XSRETURN(1); }
! #define XSRETURN_PV(v) if (0) {} else { XST_mPV(0,v); XSRETURN(1); }
! #define XSRETURN_NO if (0) {} else { XST_mNO(0); XSRETURN(1); }
! #define XSRETURN_YES if (0) {} else { XST_mYES(0); XSRETURN(1); }
! #define XSRETURN_UNDEF if (0) {} else { XST_mUNDEF(0); XSRETURN(1); }

--
Chip Salzenberg, aka <chs@nando.net>
"Hey, it's the Miss Alternate Universe Pageant!"
-- Crow T. Robot, MST3K: "Stranded In Space"

Re: Patch for XSUB.h [ In reply to ]

tye at metronet

Aug 25, 1995, 10:07 AM

Post #4 of 9 (2579 views)

Excerpts from the mail message of Douglas Lankshear:
) >+ /* Note that these are not single statements. Remember to use */
) >+ /* braces around them if needed. Don't say: if (foo) XSRETURN.. */
) > #define XSRETURN_IV(v) XST_mIV(0,v); XSRETURN(1)
) > #define XSRETURN_NV(v) XST_mNV(0,v); XSRETURN(1)
)
) Why not add the braces to the macros?
) eg:
) #define XSRETURN_IV(v) { XST_mIV(0,v); XSRETURN(1); }

I think that has to be:

#define XSRETURN_IV(v) do { XST_mIV(0,v); XSRETURN(1); } while(0)

and to avoid warnings from some compilers you have to go further:

#define NEVER (1<0)
#define XSRETURN_IV(v) do { XST_mIV(0,v); XSRETURN(1); } while(NEVER)

--
Tye McQueen tye@metronet.com || tye@doober.usu.edu
Nothing is obvious unless you are overlooking something
http://www.metronet.com/~tye/ (scripts, links, nothing fancy)

Re: Patch for XSUB.h [ In reply to ]

h.b.furuseth at usit

Aug 25, 1995, 10:29 AM

Post #5 of 9 (2577 views)

> 1. Parenthesize the asssignment macros:
>
> #define XST_mIV(i,v) (ST(i)=sv_2mortal(newSViv(v)))

And if you are going to do it right, remember that the args may need
parens too ...

#define ST(off) stack_base[ax + (off)]
#define XSRETURN_IV(v) XST_mIV(0, (v)); XSRETURN(1)

Regards,

Hallvard

Re: Patch for XSUB.h [ In reply to ]

Aug 25, 1995, 2:23 PM

Post #6 of 9 (2584 views)

Chip Salzenberg wrote:
> ! #define XSRETURN_IV(v) if (0) {} else { XST_mIV(0,v); XSRETURN(1); }

That's not the best way to do it, since it fails to compile in a context like

if (x)
XSRETURN_IV(v);
else
foo();

Either of the following will work in any statement context:

#define XSRETURN_IV(v) if (1) { XST_mIV(0,v); XSRETURN(1); } else

which may produce "null statement" warnings with some compilers, or

#define XSRETURN_IV(v) do { XST_mIV(0,v); XSRETURN(1); } while(0)

which is commonly used in FSF code.
--
<J Q B>

Re: Patch for XSUB.h [ In reply to ]

Aug 25, 1995, 2:34 PM

Post #7 of 9 (2581 views)

Hallvard B Furuseth wrote:
>
> > 1. Parenthesize the asssignment macros:
> >
> > #define XST_mIV(i,v) (ST(i)=sv_2mortal(newSViv(v)))
>
> And if you are going to do it right, remember that the args may need
> parens too ...
>
> #define ST(off) stack_base[ax + (off)]

Yup.

> #define XSRETURN_IV(v) XST_mIV(0, (v)); XSRETURN(1)

Parentheses aren't needed in that context. A precedence ambiguity could only
occur if v contains commas, but of course it can't because then it wouldn't
have been a single argument.
--
<J Q B>

Re: Patch for XSUB.h [ In reply to ]

Aug 25, 1995, 3:10 PM

Post #8 of 9 (2596 views)

According to Jim Balter:
> Chip Salzenberg wrote:
> > ! #define XSRETURN_IV(v) if (0) {} else { XST_mIV(0,v); XSRETURN(1); }
>
> That's not the best way to do it, since it fails to compile [...]:
> if (x) XSRETURN_IV(v); else foo();

Oops! "Sorry about that, Chief."

The irony is that I've used the "do { ... } while (0)" approach many
times, but I wanted to suggest something that had all its important
textual elements either to the left or to the right but not both, for
clarity. Of course, even the most clear code is worse than anything
that works... *sigh*

I shouldn't improvise when sending a message to a mailing list. :-)
--
Chip Salzenberg, aka <chs@nando.net>
"Hey, it's the Miss Alternate Universe Pageant!"
-- Crow T. Robot, MST3K: "Stranded In Space"

Re: Patch for XSUB.h [ In reply to ]

Nick.Ing-Simmons at tiuk

Aug 29, 1995, 8:30 AM

Post #9 of 9 (2581 views)

In <9508251642.AA20239@nando.net.nando.net>
On Fri, 25 Aug 1995 12:42:16 -0400 (EDT)
Chip Salzenberg <chs@nando.net> writes:
>
>! #define XSRETURN_IV(v) if (0) {} else { XST_mIV(0,v); XSRETURN(1); }
>! #define XSRETURN_NV(v) if (0) {} else { XST_mNV(0,v); XSRETURN(1); }
>! #define XSRETURN_PV(v) if (0) {} else { XST_mPV(0,v); XSRETURN(1); }
>! #define XSRETURN_NO if (0) {} else { XST_mNO(0); XSRETURN(1); }
>! #define XSRETURN_YES if (0) {} else { XST_mYES(0); XSRETURN(1); }
>! #define XSRETURN_UNDEF if (0) {} else { XST_mUNDEF(0); XSRETURN(1); }
>
I think the normal way is

#define XSRETURN_UNDEF do { XST_mUNDEF(0); XSRETURN(1); } while (0)