Fix eval_pv for old compilers #217
Closed
Conversation
This file contains 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
Macro croak_sv() is defined as STMT_START { ... } STMT_END and therefore it
cannot be used in comma operator for control flow.
So its current usage in D_PPP_CROAK_IF_ERROR macro for compilers without
support for gcc brace groups is broken and cause compile errors for code
which uses D_PPP_CROAK_IF_ERROR (e.g. eval_pv macro).
Fix this issue by converting D_PPP_CROAK_IF_ERROR macro to static inline
function. For ancient compilers without inline support, this function would
be only static, which is enough.
|
If this is called multiple times in the same compilation unit, it would be defining the same function multiple times. I think that is illegal, but maybe no. |
|
File |
atoomic
reviewed
Feb 10, 2022
| @@ -51,7 +51,8 @@ __UNDEFINED__ PERL_LOADMOD_IMPORT_OPS 0x4 | |||
| #if defined(PERL_USE_GCC_BRACE_GROUPS) | |||
| # define D_PPP_CROAK_IF_ERROR(cond) ({ SV *_errsv; ((cond) && (_errsv = ERRSV) && (SvROK(_errsv) || SvTRUE(_errsv)) && (croak_sv(_errsv), 1)); }) | |||
| #else | |||
| # define D_PPP_CROAK_IF_ERROR(cond) ((cond) && (SvROK(ERRSV) || SvTRUE(ERRSV)) && (croak_sv(ERRSV), 1)) | |||
| PERL_STATIC_INLINE void D_PPP_CROAK_IF_ERROR(int cond) { dTHX; SV *errsv; if (!cond) return; errsv = ERRSV; if (SvROK(errsv) || SvTRUE(errsv)) croak_sv(errsv); } | |||
There was a problem hiding this comment.
would adding an undef D_PPP_CROAK_IF_ERROR add some extra safety?
atoomic
approved these changes
Feb 10, 2022
|
@khwilliamson is going to run some additional extensive tests using this patch before we merge and release it in the next days. Thanks everyone |
|
On 2/10/22 14:55, ℕicolas ℝ. wrote:
@khwilliamson <https://github.com/khwilliamson> is going to run some
additional extensive tests using this patch before we merge and release
it in the next days.
Thanks everyone
—
And those tests involved making sure that we don't use brace groups at
all, and the process stopped (recent to older) at 5.9.4 with this:
'In file included from RealPPPort.xs:146:0:
',
'RealPPPort.xs: In function
‘XS_Devel__PPPort_sv_len_utf8’:
',
'ppport.h:5926:25: error: expected expression
before ‘do’
',
' # define STMT_START do
',
' ^
',
'/home/khw/devel/lib/perl5/5.9.3/x86_64-linux/CORE/sv.h:1505:23: note:
in expansion of macro ‘STMT_START’
',
' #define SvGETMAGIC(x) STMT_START { if
(SvGMAGICAL(x)) mg_get(x); } STMT_END
',
' ^
',
'ppport.h:11071:46: note: in expansion of macro
‘SvGETMAGIC’
',
' # define sv_len_utf8(sv) (PL_Sv = (sv),
SvGETMAGIC(PL_Sv), sv_len_utf8_nomg(PL_Sv))
',
' ^
',
'RealPPPort.xs:3780:26: note: in expansion of
macro ‘sv_len_utf8’
',
' RETVAL = sv_len_utf8(sv);
',
|
|
This is fixed by #218 we do not need this PR |
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.
PERL_STATIC_INLINEmacroD_PPP_CROAK_IF_ERRORwhen compiler does not support gcc brace groupsFixes: #216