beefytech
(aarch64) Duplicate-named symbol and improper resolution when using `for` loop with enumerator
(Bug found on aarch64 Switch build)
We have this function which uses a for loop with a generic enumerator Layer.ForEach<T>(). That enumerator implements public Result<T> GetNext() mut.
On aarch64, it seems this construct is causing two locals to be created with the same name:
In this example, there is one local named layer which is the enumerator itself, and a second local named layer which is the result returned from GetNext(). Unfortunately, when the local symbol layer is resolved in this scope, the compiler permits it as type T (type of the second symbol), but it is actually using the data from the enumerator (first symbol)
In this case, SetFrameBufferTexture is an Engine API function, and once it is called, the this pointer in the receiving frame is pointing at the enumerator, not the result it yielded in the for loop.
Yeah- one is the enumerator and one is the value.
It does sound like there's a calling convention issue here but it has nothing to do with the "duplicate" local variable names.
fwiw, I have a wrapper around the SetFrameBufferTexture function that is usually inlined. If I don't inline the wrapper, and step into the frame, I can see that the wrong value is returned when it tries to get &this. So maybe the problem is Beef-side, and not related to the language boundary
oh you know what... I probably need to make it do System.UnsafeCastToPtr here instead of &this...
Ah ok, I think this might be user error. This code works OK on aarch64 if I change our code generator to output System.UnsafeCastToPtr(this) instead of &this for ref type modules.
I realized we had #pragma warning disable 4204 at the top of the API bindings, specifically to let us take value of &this in struct functions that aren't marked mut. However, 4204 is the number for both of these warnings:
BF4204: Cannot take address of 'this' within struct method '...'. Consider adding 'mut' specifier to this method.
BF4204: Cannot take address of 'this' because '...' is a reference type.
The second warning was just being ignored so I didn't notice the underlying issue. So maybe there is no issue here? Maybe the second case of the warning should be separated out into its own warning number, or upgraded to error?