Skip to content

Conversation

@dfields-msft
Copy link
Contributor

Addressing #665 by promoting the array_view(pointer, size) constructor to be public rather than protected, which is consistent with C++20's std::span that winrt::array_view is designed to emulate.

@kennykerr
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@oldnewthing
Copy link
Member

Please add a unit test and verify that the deduction guide works.

@dfields-msft
Copy link
Contributor Author

@oldnewthing please let me know if the test I added is covering what you had in mind re: deduction guides.

@dfields-msft
Copy link
Contributor Author

@kennykerr this formulation requires callers to explicitly cast to size_type which is done implicitly for all other constructors. Fixing this would be possible, but would require a bit of extra template goop and I know that you're sensitive to the overhead of this. Do you have an opinion on what the contract of this constructor should be?

@oldnewthing
Copy link
Member

oldnewthing commented Jun 18, 2020

I was thinking of adding a new case to the com_array,ctad test:

    REQUIRE_DEDUCED_AS(uint8_t, { &a[0], 3 });

to verify that

com_array{ &a[0], 3 }

deduces as com_array<uint8_t>.

@dfields-msft
Copy link
Contributor Author

@oldnewthing now I see what you're talking about; done.

@oldnewthing oldnewthing self-requested a review June 18, 2020 22:44
@kennykerr
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

Copy link
Collaborator

@kennykerr kennykerr left a comment

Choose a reason for hiding this comment

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

Looks good. I just now saw your comment about size_type. C++/WinRT uses uint32_t for sizes consistent with WinRT. I'm not philosophically opposed to changing that, but it would cause ripples. Unless there's a strong reason to change it now, I'd rather hang in there until we can ditch it all in favor of std::span.

@kennykerr kennykerr merged commit 276b2f5 into microsoft:master Jun 19, 2020
@dfields-msft dfields-msft deleted the patch-3 branch June 19, 2020 23:27
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