Skip to content

This is the October 2021 Update (v2.9.0) #628

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
merged 23 commits into from
Oct 5, 2021
Merged

Conversation

Perksey
Copy link
Member

@Perksey Perksey commented Oct 2, 2021

@Perksey
Copy link
Member Author

Perksey commented Oct 2, 2021

In case it's not obvious by the "draft" status: this is not ready for review.

@Perksey Perksey changed the title BuildTools 2.9 This is the October 2021 Update (v2.9.0) Oct 3, 2021
@Perksey Perksey marked this pull request as ready for review October 3, 2021 19:27
@Perksey Perksey enabled auto-merge (squash) October 3, 2021 19:27
@Perksey
Copy link
Member Author

Perksey commented Oct 3, 2021

This is now ready for review. Meaningful changes are in src/Core/Silk.NET.BuildTools and src/Core/Silk.NET.Core

@Perksey Perksey added enhancement New feature or request feature New feature. labels Oct 3, 2021
sw.WriteLine($" public ref {field.Type} {field.Name}");
sw.WriteLine(" {");
sw.WriteLine(" [MethodImpl((MethodImplOptions) 768)]");
sw.WriteLine($" get => ref MemoryMarshal.CreateSpan(ref {args[1]}, 1)[0].{args[2]};");
Copy link
Member Author

Choose a reason for hiding this comment

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

Pre-emptive response to a potential review comment: the old code that was in the NS21+ branch didn't actually compile, and we never spotted it because for some reason the DirectX projects were NS20 only.

@Perksey Perksey added this to the 2.X milestone Oct 3, 2021
Copy link
Member

@HurricanKai HurricanKai left a comment

Choose a reason for hiding this comment

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

I can't push out the update this night anyways, and there are some issues regarding naming which we can't fix without breaking.

sw.WriteLine($" public ref {field.Type} {field.Name}");
sw.WriteLine(" {");
sw.WriteLine(" [MethodImpl((MethodImplOptions) 768)]");
sw.WriteLine($" get => ref MemoryMarshal.CreateSpan(ref {args[1]}, 1)[0].{args[2]};");
Copy link
Member

Choose a reason for hiding this comment

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

I am missing something here. How is ref CreateSpan(ref x, 1)[0] different from ref x?

Copy link
Member Author

Choose a reason for hiding this comment

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

ref x is blocked at a language level

Arguments = new()
{
"SilkTouchStage.Begin",
$"\"{last.Name} = new({outSType.DefaultAssignment});\""
Copy link
Member

Choose a reason for hiding this comment

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

Technically this can break the same people that removing the broken overloads would've broken, just saying 😛

Copy link
Member Author

Choose a reason for hiding this comment

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

The functionality of the overloads are undefined, whereas a function existing is well defined.

{
[NativeName("AnonymousName", "__AnonymousEnum_evntrace_L529_C9")]
[NativeName("Name", "ETW_COMPRESSION_RESUMPTION_MODE")]
public enum ETWCOMPRESSIONRESUMPTIONMODE : int
Copy link
Member

Choose a reason for hiding this comment

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

Naming!

Copy link
Member Author

Choose a reason for hiding this comment

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

I can’t make changes to the namer this late on - it’ll 100% change other names as well.

Copy link
Member

Choose a reason for hiding this comment

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

Well we'd see that in the diff though? If nothing else is possible I will look at the names tomorrow and manually change the ones that are all caps.

Copy link
Member Author

Choose a reason for hiding this comment

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

Special cased this

@Perksey Perksey requested a review from HurricanKai October 4, 2021 20:26
@Perksey Perksey merged commit 29dd1c6 into main Oct 5, 2021
@Perksey Perksey deleted the feature/buildtools-2.9 branch October 5, 2021 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Core/Infrastructure/Codegen bug Something isn't working enhancement New feature or request feature New feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove/Fix Vulkan "out"-binding overloads where pNext structures need to be created BuildTools work for 2.9
2 participants