Skip to content
This repository was archived by the owner on Dec 12, 2024. It is now read-only.

Redo of IteratorProtocol with tests #768

Merged
merged 5 commits into from
Sep 14, 2021
Merged

Redo of IteratorProtocol with tests #768

merged 5 commits into from
Sep 14, 2021

Conversation

stephen-hawley
Copy link
Contributor

The existing implementation was done by hand, I redid it using BTfS entirely. The old code was untested and almost assuredly would not have worked.
This code is halfway working to the extent that we can use an object that implements IteratorProtocol from swift correctly. There is an issue with sending one into swift, but fixing that now will make this (more) ungainly.

In testing this, I found that we weren't reflecting typealias declarations which are needed to understand how a class implementation a protocol with associated types binds the associated types.
I also found that we weren't binding the generics in the interface implementation of the final type, so we do that too.

{
private var _xamarinClassIsInitialized: Bool = false;

public init()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Is spacing here standard for swift? I see 2 and 4 intend?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a standard? It's likely because BTfS puts in spaces for indentation. Maybe I should change that to tabs. Maybe I'll open an issue and assign it to the next person who brings this up.

@@ -728,5 +728,78 @@ associatedtype Element
var callingCode = CSCodeBlock.Create (printIt);
TestRunning.TestAndExecute (swiftCode, callingCode, "Got here\n");
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Bunch of unrelated space diffs in this fine, but they look fine.

if (namedTypeSpec == null)
throw new NotImplementedException ("You should never get an alias name that is not a NamedTypeSpec");
var typeName = namedTypeSpec.NameWithoutModule;
if (typeName == assoc.Name || typeName == $"{proto.ToFullyQualifiedName ()}.{assoc.Name}") {
Copy link

Choose a reason for hiding this comment

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

$"{proto.ToFullyQualifiedName ()}.{assoc.Name}" will be computed several time and involve memory allocations

proto.ToFullyQualifiedName () seems to be constant in the method and could be moved out of the loops

the concatenation with assoc.Name could be moved outside the inner (alias) loop

@@ -205,6 +211,11 @@ public static TypeDeclaration TypeFromXElement (TypeAliasFolder folder, XElement
select SwiftReflector.SwiftXmlReflection.Inheritance.FromXElement (folder, inherit) as Inheritance;
decl.Inheritance.AddRange (inherits);
}
if (elem.Element ("typealiases") != null) {
Copy link

Choose a reason for hiding this comment

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

move to var ta = elem.Element ("typealiases"); and reuse later

return SwiftOptional<T>.None ();
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: missing new line!

-> SwiftIteratorProtocolProtocol<T0>
{
return SwiftIteratorProtocolProtocol();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@stephen-hawley stephen-hawley merged commit dbca458 into main Sep 14, 2021
@stephen-hawley stephen-hawley deleted the swiftsequence branch September 14, 2021 15:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants