Skip to content

Conversation

@MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Feb 20, 2022

Depends on #55.

I'm not adding the default (and only) implementation here yet, as it crashes the type checker with Assertion failed: (protocol && "requirements should have stopped recursion") error message:

extension SignedNumeric {
  @_transparent
  public static prefix func - (_ operand: Self) -> Self {
    var result = operand
    result.negate()
    return result
  }

  @_transparent
  public mutating func negate() {
    self = 0 - self
  }
}

@MaxDesiatov MaxDesiatov changed the title Add SignedNumeric protocol Core: add SignedNumeric protocol Feb 20, 2022
@MaxDesiatov MaxDesiatov marked this pull request as ready for review February 20, 2022 20:51
@compnerd
Copy link
Owner

Interesting, sounds like we are missing some requirements that the compiler expected? We should try to identify what those requirements are at the very least before we merge this I think.

@MaxDesiatov MaxDesiatov marked this pull request as draft February 20, 2022 21:30
@MaxDesiatov
Copy link
Contributor Author

A lot of numerical stuff requires a huge amount of operators and their dependencies introduced at once. I had to omit some of those to split it into separate PRs. Would you find it acceptable if I don't omit anything and submit things without breaking these cyclic dependencies? That would result in huge PRs.

@compnerd
Copy link
Owner

I think that if we know/understand what is missing and having that as FIXMEs is fine. We need to break the dependency cycle somewhere and I think that doing that iteratively is going to be easier.

@MaxDesiatov
Copy link
Contributor Author

Main blocker for me here is associatedtype Words: RandomAccessCollection on BinaryInteger, and both of those depend on Strideable, which depends on SignedNumeric. I could add a FIXME in this PR, saying that default implementation for negate() and prefix func - is missing. Would that be ok?

@compnerd
Copy link
Owner

Perfet! At least we understand what is missing and why, so I don't see that as a blocker for making progress.

@MaxDesiatov MaxDesiatov marked this pull request as ready for review February 20, 2022 21:48
@MaxDesiatov
Copy link
Contributor Author

Great, that's ready now.

@compnerd compnerd merged commit e3909ab into compnerd:main Feb 20, 2022
@MaxDesiatov MaxDesiatov deleted the signed-numeric branch February 20, 2022 23:45
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.

2 participants