Uh oh!
There was an error while loading. Please reload this page.
Fix COMDelegate::BindToMethod with static virtuals - #125875
Conversation
The code was incorrectly using VSD stubs for them which led to runtime asserts.
jkotas
commented
Mar 21, 2026
Do we need a test? |
MichalPetryka
commented
Mar 21, 2026
Added a test via reflection, but I'm not sure if you should be able to CreateDelegate like this at all (I hit the asserts while working on another PR that hit it differently). |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
I think it should work. @MichalStrehovsky Do you agree? |
Removed TODO comment regarding VSD compilation and virtuals.
MichalPetryka
commented
Mar 21, 2026
@MichalStrehovsky Should I mark the test as skipped in NAOT or should it understand the case here? |
MichalStrehovsky
commented
Mar 23, 2026
If there's no trim/AOT warning, the code should work with NAOT. There was a TODO that was unnecessarily blocking this, I pushed out a change to remove it (we still don't allow invoking these virtually but that's because there's no reflection API for that; we'd fix that if there ever is an API). |
jkotas
commented
Mar 23, 2026
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Removed comments about NativeAOT behavior and kept attributes.
Uh oh!
There was an error while loading. Please reload this page.
MichalStrehovsky
commented
Mar 23, 2026
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
MichalPetryka
commented
Mar 24, 2026
@MichalStrehovsky The assertion failure in NAOT after your changes seem related, do you want to look into it yourself or should I try to do it myself? |
The assert is only trying to approximate the thing it's asserting since we lack the API to check "is this method a MethodImpl?".
MichalStrehovsky
commented
Mar 25, 2026
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
MichalPetryka
commented
Mar 25, 2026
NAOT failures seem unrelated now, is this good to merge? |
MichalStrehovsky
left a comment
There was a problem hiding this comment.
LGTM but I did half of the change :)
jkotas
commented
Mar 26, 2026
@MichalStrehovsky Are the test build breaks known or already fixed by some other PR? |
MichalStrehovsky
commented
Mar 26, 2026
They are known, the Pri1 test tree is not buildable (not specific to native AOT): #126108 (comment) |
MichalStrehovsky
commented
Mar 26, 2026
/ba-g the pri1 test run looked good in the previous outerloop iteration and we only relaxed an assert since then. the pri1 break is known. |
The code was incorrectly using VSD stubs for them which led to runtime asserts.