Skip to content

Conversation

@oldnewthing
Copy link
Member

@oldnewthing oldnewthing commented Jun 8, 2020

In a call to unbox_value_or<T>(obj, def), if the second parameter happens to be a valid constructor parameter for param::hstring, then the hstring version of unbox_value_or will be used, even though the user clearly wants T.

// expected to unbox obj to an IPropertyValue,
// but actually unboxes to an hstring!
auto pv = unbox_value_or<IPropertyValue>(obj, {});

Add an explicit enable_if to remove the hstring version if the template parameter is not hstring.

There's the converse problem, where you want unbox_value_or to infer hstring, but it can't:

// Doesn't compile due to inability to infer T.
auto str = unbox_value_or(obj, hstring{});

Add a default type hstring for T in the hstring version of unbox_value_or.

In a call to `unbox_value_or<T>(obj, def)`, if the
second parameter happens to be a valid constructor
parameter for `param::hstring`, then the hstring
version of `unbox_value_or` will be used, even though
the user clearly wants `T`.

```cpp
// expected to unbox obj to an IPropertyValue,
// but actually unboxes to an hstring!
auto pv = unbox_value_or<IPropertyValue>(obj, {});
```

Add an explicit `enable_if` to remove the hstring
version if the template parameter is not `hstring`.

There's the converse problem, where you want
`unbox_value_or` to infer `hstring`, but it can't:

```cpp
// Doesn't compile due to inability to infer T.
auto str = unbox_value_or<hstring>(obj, hstring{});
```

Add a default type `hstring` for `T` in the hstring
version of `unbox_value_or.
@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.

Thanks!

@kennykerr kennykerr merged commit f816245 into microsoft:master Jun 8, 2020
@oldnewthing oldnewthing deleted the ambiguous-unbox branch June 8, 2020 15:34
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.

2 participants