Skip to content

Conversation

@manodasanW
Copy link
Member

  • Added support for projecting classes implementing generics
  • Added support for projecting arrays
  • Ported all the additions to the new format.
  • To avoid having them as all manual projections, I added support for partial additions which basically cswinrt will generate the type with partial and we will add the addition as another partial type. This is mostly for types which just had a custom ToString that they wanted to add.
  • Enabled all the projections and address issues encountered.
  • Moved most event sources to respective ABI namespace to avoid hard coding in cswinrt.exe

Copy link
Member

@Sergio0694 Sergio0694 left a comment

Choose a reason for hiding this comment

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

Did a first partial pass, left some notes for now 🙂

{
public override string ToString()
{
return string.Concat("GeneratorPosition (", Index.ToString(global::System.Globalization.CultureInfo.InvariantCulture), ",", Offset.ToString(global::System.Globalization.CultureInfo.InvariantCulture), ")");
Copy link
Member

Choose a reason for hiding this comment

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

We can optimize this with an interpolated string builder. I'll do this in a separate PR, want to experiment with exposing a custom handler from the runtime so that we can reuse it in all these places where we just always do invariant culture formatting.


case RepeatBehaviorType.Count:

global::System.Text.StringBuilder sb = new global::System.Text.StringBuilder();
Copy link
Member

Choose a reason for hiding this comment

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

Also we should set SkipLocalsInit for all projection .dlls if we don't already. At least the merged .dll, as we own it.

// the transform is identity by default
private static Matrix s_identity = CreateIdentity();

public static Matrix Identity
Copy link
Member

Choose a reason for hiding this comment

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

@tannergooding is there something smart we could do here like creating a BCL matrix and reinterpreting it or something, rather than storing this value in a static field like this?


w.write(R"(
%IEnumerator<%> %GetEnumerator() => %.GetEnumerator(%);
[UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name = "GetEnumerator")]
static extern IEnumerator<%> %GetEnumerator([UnsafeAccessorType("ABI.System.Collections.Generic.<#CsWinRT>IEnumerableMethods`1<<#corlib>System-ComponentModel-DataErrorsChangedEventArgs>, WinRT.Interop.dll")] object _, WindowsRuntimeObjectReference objRef);
Copy link
Member

Choose a reason for hiding this comment

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

Generic mangled names use ' for the generic arity prefix, not the backtick (we changed it as it broke ILSpy) 😄

_m31, _m32, _m33, _m34,
_offsetX, _offsetY, _offsetZ, _m44);
char separator = global::WindowsRuntime.InteropServices.TokenizerHelper.GetNumericListSeparator(provider);
DefaultInterpolatedStringHandler handler = new(0, 31, provider, stackalloc char[64]);
Copy link
Member

Choose a reason for hiding this comment

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

Not entirely sure 64 is enough, can we double check what's the average length of this text and adjust the scratch buffer? If it's 256 or less we can use that, otherwise we can skip it entirely if it won't be enough anyway, to avoid the extra copy.

OffsetX,
OffsetY);
char separator = global::WindowsRuntime.InteropServices.TokenizerHelper.GetNumericListSeparator(provider);
DefaultInterpolatedStringHandler handler = new(0, 11, provider, stackalloc char[64]);
Copy link
Member

Choose a reason for hiding this comment

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

Same here as above, we might want to tweak the scratch buffer size.

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