Skip to content

Conversation

@khwilliamson
Copy link
Contributor

@khwilliamson khwilliamson commented Aug 21, 2025

PERL_UNIQUE_NAME is a factory creating a preprocessor symbol that is based on an input skeleton name, and the line number it is called from. The skeleton name would likely be something like sv1_ or PL_sv1. These should be chosen to not have conflicts with the XS code. The advantage of this approach is that you won't be using the same variable name in a bunch of macros and they collide.

  • This set of changes does not require a perldelta entry.

@Leont
Copy link
Contributor

Leont commented Aug 21, 2025

Both of these make me wonder if they wouldn't be better off as an inline function.

@jkeenan
Copy link
Contributor

jkeenan commented Sep 2, 2025

Both of these make me wonder if they wouldn't be better off as an inline function.

@khwilliamson could you respond to the above? Thanks.

This macro acts like a factory to produce a name from its argument that
also includes the line number.
This uses the PERL_UNIQUE_NAME() macro created in the previous commit to
get unique names for variables in macro expansions.

Suppose you have

    #define foo STMT_START { ... } STMT_END

block, and have a variable 'bar' declared in it.  By first saying

    #define bar PERL_UNIQUE_NAME(bar)

'bar' will be replaced in the expansion of 'foo' by a unique value.

It is your responsibility to not have two 'bar' symbols in the same
program.  But if you forget, you will be warned by the compiler that
'bar' is redefined, so you can fix it
.
@khwilliamson khwilliamson changed the title Change names in macros to avoid possible collisions Create PERL_UNIQUE_NAME and use it to avoid name collisions Sep 3, 2025
@khwilliamson
Copy link
Contributor Author

There is a difference of opinion between people who like macros, and those who like inline functions. I was always taught that inline functions were more precious than macros, so should be avoided. No one has ever said to me that this is wrong nowadays.

So, I've changed this from the original commit, by using a factory macro to spew out unique names. And, I've only converted one macro to use it.

My principal objection to using macros over inline functions is that there can be name collisions with the arguments, and to avoid this, the names that are declared inside the macro get long and unwieldy; hard to read.

This p.r. pretty much makes that a non-issue. You can create your own short name using good naming practices, and it will expand to something unique, and if you collide with another short name, the compiler will warn you.

My principal objection to using inline functions over macros, is that I was taught these were a precious commodity; that most compilers had a very finite limit as to how many it would actually look at and inline. I think this p.r. solves that

@khwilliamson khwilliamson merged commit c4da28b into Perl:blead Sep 11, 2025
33 checks passed
@tonycoz
Copy link
Contributor

tonycoz commented Oct 30, 2025

I noticed this when looking for the OpTYPE_set() source (looking at @scottchiefbaker's uinteger) - it has the same problem as was mentioned elsewhere, o1_ and type1_ are replaced globally, if someone is using a trailing underscore for structure members, this is going to replace them with line specific nonsense.

You could handle this by using two macros, something like (untested):

#define OpTYPE_set_(o,type, o1, type1)                      \
    STMT_START {                                \
        OP *o1 = (OP *)o;                      \
        OPCODE type1 = type;                   \
        o1_->op_type = type1;                  \
        o1_->op_ppaddr = PL_ppaddr[type1];     \
    } STMT_END

#define OpTYPE_set(o, type) OpTYPE_set_(o, type, PERL_UNIQUE_NAME(o_temp), PERL_UNIQUE_NAME(type_temp))

Doing this limits the visibility of the replacement to the macro body.

Or we can use an inline function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants