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

Fix constraint in Probe #14

Merged
merged 2 commits into from
Jan 16, 2025
Merged

Fix constraint in Probe #14

merged 2 commits into from
Jan 16, 2025

Conversation

bioball
Copy link
Contributor

@bioball bioball commented Jan 15, 2025

Add grpc to the set of possible properties

Add grpc to the set of possible properties
@bioball bioball requested a review from madrob January 15, 2025 22:53
@@ -131,6 +131,10 @@ const function hasNonNullPortNames(l: Listing): Boolean = (l.length > 1).implies
const function exactlyOneSet(a: Any, b: Any, c: Any) =
(a != null).xor(b != null).xor(c != null) && !(a != null && b != null && c != null)

/// Tells if exactly one of the four arguments is non-null.
const function exactlyOneSet4(a: Any, b: Any, c: Any, d: Any) =
(a != null).xor(b != null).xor(c != null).xor(d != null) && !(a != null && b != null && c != null && d != null)
Copy link

Choose a reason for hiding this comment

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

I think this will return true if 3/4 are true

Copy link

Choose a reason for hiding this comment

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

pkl1> a = null; b = "x" ; c = 2 ; d = true
pkl2> (a != null).xor(b != null).xor(c != null).xor(d != null) && !(a != null && b != null && c != null && d != null)
true

Copy link
Contributor Author

@bioball bioball Jan 16, 2025

Choose a reason for hiding this comment

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

Good catch, updated.

I don't know if there's a simpler way to write this expression asides from my updated version; feel free to provide a suggestion

Copy link

Choose a reason for hiding this comment

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

You could do a summation of a != null ? 1 : 0 + ... == 1 but I'm not sure if that comes out simpler or more readable

Comment on lines +134 to +140
/// Tells if exactly one of the four arguments is non-null.
const function exactlyOneSet4(a: Any, b: Any, c: Any, d: Any) =
(a != null && b == null && c == null && d == null)
|| (a == null && b != null && c == null && d == null)
|| (a == null && b == null && c != null && d == null)
|| (a == null && b == null && c == null && d != null)

Choose a reason for hiding this comment

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

Suggested change
/// Tells if exactly one of the four arguments is non-null.
const function exactlyOneSet4(a: Any, b: Any, c: Any, d: Any) =
(a != null && b == null && c == null && d == null)
|| (a == null && b != null && c == null && d == null)
|| (a == null && b == null && c != null && d == null)
|| (a == null && b == null && c == null && d != null)
/// Tells if exactly one of the arguments is non-null.
local function exactlyOneSet(args: List<Any>): Boolean =
args.fold(0, (i, arg) -> if (arg != null) i + 1 else i) == 1

You can remove the other exactlyOneSet and only have this one.
This would be a prime candidate for varargs once we support it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather avoid the allocation here. But, we should really have better support for mutual exclusion; maybe a good candidate to add to pkl:base

@bioball bioball merged commit 583e31a into apple:main Jan 16, 2025
3 checks passed
@bioball bioball deleted the fix-probe-constraint branch January 16, 2025 14:13
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.

3 participants