Skip to content

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.

4 participants