Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zefram/cmpchain: 1 < $y < 10 #17547

Merged
merged 4 commits into from Mar 13, 2020
Merged

Zefram/cmpchain: 1 < $y < 10 #17547

merged 4 commits into from Mar 13, 2020

Conversation

demerphq
Copy link
Collaborator

@demerphq demerphq commented Feb 8, 2020

Adds support for chained relational and equivalence operators.

Allows

if (1 < $y < 10) { ... }

type comparisons.

Zefram and others added 4 commits February 8, 2020 12:13
Warn explicitly that chainable operators don't chain with operators of
different precedence.
Optype appears to be almost completely unused, and on Win32 builds
we saw warnings from the cmpchain patches:

    perly.y(1063) : warning C4244: 'function' : conversion from 'I32' to 'Optype', possible loss of data
    perly.y(1065) : warning C4244: 'function' : conversion from 'I32' to 'Optype', possible loss of data
    perly.y(1079) : warning C4244: 'function' : conversion from 'I32' to 'Optype', possible loss of data
    perly.y(1081) : warning C4244: 'function' : conversion from 'I32' to 'Optype', possible loss of data

Reviewing the code I noticed that functions like Perl_newBINOP() have
an I32 type argument, and functions like OpTYPE_set() coerce such
arguments into type OPCODE:

	#define OpTYPE_set(o,type) \
	STMT_START {                                \
	    o->op_type = (OPCODE)type;              \
	    o->op_ppaddr = PL_ppaddr[type];         \
	} STMT_END

this patch changes the signature to the new cmpchain functions so that
they do they same, and change the type for storage for op_type values
to also use OPCODE like most of the other op.c code.
@atoomic
Copy link
Member

atoomic commented Mar 13, 2020

@xsawyerx any ideas which version we should target for this feature?

@atoomic atoomic changed the title Zefram/cmpchain Zefram/cmpchain: 1 < $y < 10 Mar 13, 2020
@khwilliamson khwilliamson merged commit 6b32f57 into blead Mar 13, 2020
@khwilliamson
Copy link
Contributor

Sawyer already said he wanted it in. There is a little debate about whether it should have an experimental warning or not. But I thought it best to get it in as soon as possible.

perldelta should be updated when we finally decide

@rjbs rjbs deleted the zefram/cmpchain branch May 8, 2021 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants