Skip to content

!feat: change zip underlying implementation#28

Open
JaapWijnen wants to merge 1 commit into
mainfrom
test/change-zip-implementation
Open

!feat: change zip underlying implementation#28
JaapWijnen wants to merge 1 commit into
mainfrom
test/change-zip-implementation

Conversation

@JaapWijnen

Copy link
Copy Markdown
Collaborator

This simplifies the protocol requirements for DifferentiableSequence but breaks the previous api (people shouldn't have adopted the protocol but it can't be kept internal). The change enables us to conform StencilCollection to DifferentiableCollection.

This simplifies the protocol requirements for DifferentiableSequence but breaks the previous api (people shouldn't have adopted the protocol but it can't be kept internal). The change enables us to conform StencilCollection to DifferentiableCollection.
@JaapWijnen JaapWijnen requested a review from tmcdonell February 4, 2026 14:38
@tmcdonell

Copy link
Copy Markdown
Contributor

breaks the previous api (people shouldn't have adopted the protocol but it can't be kept internal)

You mean the change from appendContribution(of:) to writeTangentContribution(of:at:)? I would also consider that an internal/private interface that is not meant to be used by people, so I think that's fine to change.

Nit: switch the order of the arguments to match initializeElement(at:to:)? And is the consuming annotation applicable to add here as well? (not sure, honest question)

@JaapWijnen

Copy link
Copy Markdown
Collaborator Author

switch the order of the arguments to match initializeElement(at:to:)

Oh great suggestion!

And is the consuming annotation applicable to add here as well? (not sure, honest question)

I'm not sure what you're referencing here exactly. Could you elaborate?

@tmcdonell

Copy link
Copy Markdown
Contributor

I'm not sure what you're referencing here exactly. Could you elaborate?

The type signature for initializeElement(at:to:) has this consuming annotation; not sure what that does but was wondering if it's useful for us here as well.

func initializeElement(
    at index: UnsafeMutableBufferPointer<Element>.Index,
    to value: consuming Element
)

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