-
Notifications
You must be signed in to change notification settings - Fork 513
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
More variant types #3146
More variant types #3146
Conversation
`is_empty` was already marked so but could not be used as such since there was no way to safely build a value of the type that was indeed empty. In order to be consistent, mark the rest as `const` too. Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
@microsoft-github-policy-service agree company="HarfangLab" |
Thanks, I'd like to reduce rather than expand |
Alright, ping me when you do 🙂 |
? |
We'll keep the discussion alive on the issue. 😊 |
Actually, I just thought about this again: why block this PR's review, iteration and potential merge while waiting for a refactor you want to achieve? The added APIs would still remain after the refactor, whether they are added before or after, right? Also, in any case, moving the types to a separate crate could potentially constitute an API break, but that is independent of the things proposed here, right? This and that can be done in parallel, no? |
Dear maintainers,
This is an attempt to close #2983. It is effectively an upstream of remainder of the equivalent higher-level wrapper we have around
VARIANT
.It contains:
VT_NULL
variant.is_null
was also added as a bonus for symmetry withis_empty
.From<String>
mapping toVT_BSTR
sinceBSTR
also implements it.From<&[&str]>
mapping toVT_ARRAY | VT_BSTR
sinceBSTR
implementsFrom<&str>
.From<&[String]>
mapping toVT_ARRAY | VT_BSTR
sinceBSTR
also implementsFrom<&String>
.From<&[u8]>
mapping toVT_ARRAY | VT_UI1
.The array variants require allocation that is hopefully handled correctly. The
Drop
implementations were updated accordingly.Also,
is_empty
wasconst
, so I thought it would be interesting as a sort of bonus to mark the other methods as such: see first commit. I can remove it though, if it is judged not acceptable.Cheers,
Paul.