Conversation
0a27223 to
34eff30
Compare
|
@filmor If I understand this correctly, one wouldn't even need the dynamic.py script anymore for DynamicObject access to work, correct? That seems great :) |
|
@spkl Yes, the dynamic support would be integrated. To build a wheel, the simplest way is to get |
|
OK so I did a little digging and found out that I can Getting the value of dynamic properties seems to work, but I encountered a problem when setting dynamic properties. This is what I observed when running a script like this: x = obj.MyProp
obj.MyProp = None
y = obj.MyPropFor line 1, |
| return default; | ||
| } | ||
|
|
||
| if (GetManagedObject(ob) is not CLRObject co) | ||
| { | ||
| return default; | ||
| } | ||
|
|
||
| // Slot registration already guarantees this type supports DLR | ||
| var dynamicObject = (IDynamicMetaObjectProvider)co.inst; | ||
|
|
||
| string? memberName = Runtime.GetManagedString(key); | ||
| if (memberName is null) | ||
| { | ||
| return default; |
There was a problem hiding this comment.
These return defaults are a little concerning. We are basically hiding errors (which I am not sure can even happen in the first place).
There was a problem hiding this comment.
We are not. If we arrive at any of these, we have already gotten an error from the initial Python getattr. Since we are not clearing it until the very end, this is what Python is going to see. I'll add some more tests for that.
| return -1; | ||
| } | ||
|
|
||
| // Deletion fallback is intentionally not handled by DLR binder yet. |
There was a problem hiding this comment.
"yet" here means we can't fix it until 4.0 as it would be breaking change
| } | ||
|
|
||
| // Deletion fallback is intentionally not handled by DLR binder yet. | ||
| if (val == null) |
There was a problem hiding this comment.
I don't remember if BorrowedReference == null is implemented. It might be better to explicitly use IsNull
| return -1; | ||
| } | ||
|
|
||
| if (GetManagedObject(ob) is not CLRObject co) | ||
| { | ||
| return -1; | ||
| } | ||
|
|
||
| // Slot registration already guarantees this type supports DLR. | ||
| var dynamicObject = (IDynamicMetaObjectProvider)co.inst; | ||
|
|
||
| string? memberName = Runtime.GetManagedString(key); | ||
| if (memberName is null) | ||
| { | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
Same as above, silent error masking
| TypeManager.InitializeSlotIfEmpty(pyType, TypeOffset.tp_getattro, new Interop.BB_N(tp_getattro_dlr), slotsHolder); | ||
| TypeManager.InitializeSlotIfEmpty(pyType, TypeOffset.tp_setattro, new Interop.BBB_I32(tp_setattro_dlr), slotsHolder); |
There was a problem hiding this comment.
Are there any conflicts possible?
Implements #72 and should thus fix #2696.
@spkl Could you try out this branch?