Skip to content

stdlib: Add SIMDMaskScalar.SIMDMaskScalar == SIMDMaskScalar requirement to SIMDScalar #59647

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

Merged

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Jun 22, 2022

In theory this is a source break if someone had a weird custom
conforming type, but I suspect in practice conformances to
this protocol are never defined.

The reason we want this requirement is that often you will see
code like the following:

  protocol Point {
    associatedtype Scalar: SIMDScalar
    associatedtype Vector: SIMD where Vector.Scalar == Scalar
  }

  extension Point where Vector == SIMD2<Scalar> { ... }

When Vector is equated with SIMD2<Scalar>, we get an infinite
sequence of implied same-type requirements:

  Vector.MaskStorage == SIMD2<Scalar.MaskScalar>
  Vector.MaskStorage.MaskStorage == SIMD2<Scalar.MaskScalar.MaskScalar>
  ...

The protocol fails to typecheck with an error because the requirement
machine cannot build a rewrite system.

If SIMDScalar requires that MaskScalar.MaskScalar == MaskScalar, then
we instead get

  Vector.MaskStorage == SIMD2<Scalar.MaskScalar>
  Vector.MaskStorage.MaskStorage == SIMD2<Scalar.MaskScalar>

  Vector.MaskStorage.MaskStorage == Vector.MaskStorage
  ...

Which ties off the recursion.

In theory, a more advanced implementation could represent this kind of
infinite recursion in 'closed form', but we don't have that yet, and I
believe adding this same-type requirement makes sense anyway.

Fixes rdar://problem/95075552.

@slavapestov slavapestov force-pushed the stdlib-simd-requirement-machine-hack branch 2 times, most recently from 704850e to 158d8e1 Compare June 22, 2022 17:16
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

…nt to SIMDScalar

In theory this is a source break if someone had a weird custom
conforming type, but I suspect in practice conformances to
this protocol are never defined.

The reason we want this requirement is that often you will see
code like the following:

  protocol Point {
    associatedtype Scalar: SIMDScalar
    associatedtype Vector: SIMD where Vector.Scalar == Scalar
  }

  extension Point where Vector == SIMD2<Scalar> { ... }

When `Vector` is equated with `SIMD2<Scalar>`, we get an infinite
sequence of implied same-type requirements:

  Vector.MaskStorage == SIMD2<Scalar.MaskScalar>
  Vector.MaskStorage.MaskStorage == SIMD2<Scalar.MaskScalar.MaskScalar>
  ...

The protocol fails to typecheck with an error because the requirement
machine cannot build a rewrite system.

If SIMDScalar requires that MaskScalar.MaskScalar == MaskScalar, then
we instead get

  Vector.MaskStorage == SIMD2<Scalar.MaskScalar>
  Vector.MaskStorage.MaskStorage == SIMD2<Scalar.MaskScalar>

  Vector.MaskStorage.MaskStorage == Vector.MaskStorage
  ...

Which ties off the recursion.

In theory, a more advanced implementation could represent this kind of
infinite recursion in 'closed form', but we don't have that yet, and I
believe adding this same-type requirement makes sense anyway.

Fixes rdar://problem/95075552.
@slavapestov slavapestov force-pushed the stdlib-simd-requirement-machine-hack branch from 158d8e1 to 2734f3c Compare June 23, 2022 01:26
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

1 similar comment
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

@slavapestov slavapestov merged commit 038670c into swiftlang:main Jun 23, 2022
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.

1 participant