Skip to content

Conversation

@dfields-msft
Copy link
Contributor

Make winrt::guid("...") more useful at runtime by throwing on failure instead of aborting the program. See #991

Make `winrt::guid("...")` more useful at runtime by throwing on failure instead of aborting the program.
@dfields-msft
Copy link
Contributor Author

@kennykerr does a successful run of "C++ for WinRT Xlang Internal Build" include the Clang validation you were asking for?

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.

Thanks! Please add a test that verifies this works. At a glance, this won't compile.

@kennykerr
Copy link
Collaborator

Yes, you can either validate it locally or run the build once a test has been added.

@kennykerr
Copy link
Collaborator

Oh and the CI build for some reason will only run branches local to this repo.

@dfields-msft
Copy link
Contributor Author

Does the test infrastructure have any existing support for compile-time negative tests (i.e. code that's expected to not compile)? I can certainly add compile-time and runtime positive tests, as well as a runtime negative test, but I'm not going to bite off setting up test infrastructure for compile-time negative tests if that's not already available...

@kennykerr
Copy link
Collaborator

No negative testing unfortunately.

Just need a test that validates it works in a constant expression (e.g. static_assert) and a runtime test that validates it works successfully (e.g. assert) and unsuccessfully (e.g. throw and catch).

@dfields-msft
Copy link
Contributor Author

@kennykerr I'm not sure how to trigger the "C++ for WinRT Xlang internal build" pipeline run that's required by policy... Is this even possible, since my changes are coming from a different fork? I don't think I have permission to create a branch in the official microsoft fork.

@kennykerr
Copy link
Collaborator

I've given you Write access so you should be able to push a branch. Then you can go here and build your branch:

https://dev.azure.com/microsoft/Dart/_build?definitionId=31784

@dfields-msft
Copy link
Contributor Author

Verified offline that Clang works with these changes too. @kennykerr any additional changes needed, or is this ready to merge?

@dfields-msft dfields-msft linked an issue Aug 3, 2021 that may be closed by this pull request
@dfields-msft
Copy link
Contributor Author

Thanks for approving the changes, @kennykerr! Looks like I'm not authorized to merge, so you may need to complete this PR yourself...

@kennykerr
Copy link
Collaborator

@DefaultRyan or @jlaanstra can you help? I'm on the beach. 😎

@jlaanstra jlaanstra merged commit 4ba22a6 into microsoft:master Aug 4, 2021
@kennykerr kennykerr mentioned this pull request Sep 1, 2021
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.

guid parse errors can't be caught and handled at runtime

4 participants