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

Set correct priority for ?: operator #12075

Merged
merged 1 commit into from
Mar 19, 2020

Conversation

DamirAinullin
Copy link
Contributor

@DamirAinullin DamirAinullin commented Mar 8, 2020

PR Summary

PR Context

There is an operator precedence error in Binders.cs.
Because the '&&' operator's priority is higher than that of '?:', the 'x.Item1.Equals(y.Item1) && x.Item2 == null' expression will be calculated at first. To fix such behavior I suggest to add parentheses for '?:' expression.

PR Checklist

@ghost ghost assigned adityapatwardhan Mar 8, 2020
@daxian-dbw
Copy link
Member

@DamirAinullin Can you please open an issue first to better describe the issue?

@DamirAinullin
Copy link
Contributor Author

@daxian-dbw this is just code-reated problem, incorrect operator priority. I don't know what else should I describe in issue.

@daxian-dbw
Copy link
Member

@DamirAinullin A repro would be helpful to understand the problem. Also, for a bug, it's recommended to open an issue before submitting a PR.

@DamirAinullin
Copy link
Contributor Author

@daxian-dbw I don't have the reproducing case.

@daxian-dbw
Copy link
Member

@DamirAinullin My apology! I thought it was the chain operator and ternary operator introduced into PowerShell 7, and didn't realize you were taking about the C# operators. For subtle things like this one, no need to open an issue. Your changes look good to me.

Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Mar 19, 2020
@iSazonov iSazonov added this to the 7.1.0-preview.1 milestone Mar 19, 2020
@iSazonov iSazonov merged commit 5da0697 into PowerShell:master Mar 19, 2020
@iSazonov
Copy link
Collaborator

@DamirAinullin Thanks for your contribution!

@DamirAinullin DamirAinullin deleted the wrong_operator_priority branch March 19, 2020 10:57
@ghost
Copy link

ghost commented Mar 26, 2020

🎉v7.1.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants