Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[VisualBasic] Fix TypeName for COM objects #40584

Merged
merged 12 commits into from
Aug 11, 2020
Merged

[VisualBasic] Fix TypeName for COM objects #40584

merged 12 commits into from
Aug 11, 2020

Conversation

ehasis
Copy link
Contributor

@ehasis ehasis commented Aug 9, 2020

Fix Microsoft.VisualBasic.Information.TypeName to return correct name for COM objects

  • Added necessary types in UnsafeNativeMethods (i.e. IDispatch, ITypeInfo, etc.)
  • Completed the method LegacyTypeNameOfCOMObject to match reference source
  • Added the method TypeNameOfCOMObject and call it from Versioned.TypeName

Caveats

  • The implementation is Windows specific
  • Added the file LLinkTrim.xml to prevent the Linker from removing "unused" methods and breaks the vtable layouts
  • The tests instantiates COM objects from some common libraries finds in Windows

Resolves #35937

@ehasis
Copy link
Contributor Author

ehasis commented Aug 9, 2020

Not sure why the tests are breaking on coreclr scenario. On mono is because PlatformNotSupportedException in Type.GetTypeFromProgID. Any suggestions in how to treat this?

@danmoseley danmoseley requested a review from cston August 11, 2020 00:19
@ghost
Copy link

ghost commented Aug 11, 2020

Tagging subscribers to this area: @cston
See info in area-owners.md if you want to be subscribed.

@danmoseley
Copy link
Member

danmoseley commented Aug 11, 2020 •
edited
Loading

On mono is because PlatformNotSupportedException in Type.GetTypeFromProgID

@AaronRobinsonMSFT it appears we do not have any tests in the repo for Type.GetTypeFromProgID, does that seem right? I see VB has Interaction.CreateObject that uses it, but tests for that just pass error cases.

@ehasis you can apply [SkipOnMono("COM Interop not supported on Mono")] to the test.

@steveisok weren't we going to add SupportsComInterop = IsMonoRuntime; in PlatformDetection for cases like this? I don't see anything like that so I guess it just wasn't needed yet?

cston
cston approved these changes Aug 11, 2020
View reviewed changes
Copy link
Member

@cston cston left a comment

Choose a reason for hiding this comment

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

Thanks @ehasis!

eerhardt
eerhardt approved these changes Aug 11, 2020
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

The ILLinkTrim.xml file looks good to me.

@danmoseley danmoseley merged commit 8bc5dbd into dotnet:master Aug 11, 2020
@danmoseley
Copy link
Member

Thanks for the contribution @ehasis

@ehasis ehasis deleted the vb-typename branch August 11, 2020 23:53
@karelz karelz added this to the 5.0.0 milestone Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@danmoseley danmoseley danmoseley left review comments

@cston cston cston approved these changes

@eerhardt eerhardt eerhardt approved these changes

Assignees
No one assigned
Labels
area-Microsoft.VisualBasic
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

Microsoft.VisualBasic.Information.TypeName returns wrong name for COM objects
6 participants
@ehasis @danmoseley @eerhardt @cston @karelz @Dotnet-GitSync-Bot