X-Sharp / XSharpPublic

Public repository for the source code for the XSharp Compiler, Runtime, Project System and Tools.
Apache License 2.0
112 stars 38 forks source link

Incorrect "assignment" behavior (XBase++) #1491

Open DenGhostYY opened 3 months ago

DenGhostYY commented 3 months ago

Describe the bug The compiler throws an error due to assignment of a "readonly" field in the context of a method of the same class.

To Reproduce

procedure main()
    local o := Example():new(4)

    // Error BASE/Код 2246;nNumber;Access to member-variable not allowed in this context
    o:nNumber := 10
    ? o:nNumber
    o:SetNumber(15)
    ? o:nNumber
return

class Example
exported:
    var nNumber assignment hidden

    inline method init(nNumber)
        ::nNumber := nNumber
    return

    inline method SetNumber(nNumber)
        ::nNumber := nNumber
    return self
endclass

Expected behavior (XBase++ exe) Error

Error BASE/Код 2246;nNumber;Access to member-variable not allowed in this context

Actual behavior (X# exe) No compilation errors or warnings

10
15

Additional context X# Compiler version 2.20.0.3 (public) -dialect:xBase++ -codepage:866 -lb -enforceself -memvar -xpp1 -vo1 -vo3 -vo4 -vo5 -vo9 -vo10 -vo12 -vo13 -vo14 -vo15 -vo16 -vo17 -reference:XSharp.Core.dll -reference:XSharp.RT.dll -reference:XSharp.XPP.dll

RobertvanderHulst commented 3 months ago

We will change this and implement it just like C# does: only assignments in the constructor will be allowed

DenGhostYY commented 3 months ago

If you implement this as in C#, then a lot of legacy Alaska code will not compile on XSharp. We planned to replace the readonly (if there are any difficulties with it) with the assignment hidden|protected.

RobertvanderHulst commented 3 months ago

If you implement this as in C#, then a lot of legacy Alaska code will not compile on XSharp. We planned to replace the readonly (if there are any difficulties with it) with the assignment hidden|protected.

Why would that not work?

DenGhostYY commented 3 months ago

Because at the moment class fields marked with readonly can only be assigned inside a constructor, but not inside methods of the same class, although Alaska allows this.

DenGhostYY commented 3 months ago

This ticket shows exactly this difference in the behavior of the keyword readonly

RobertvanderHulst commented 3 months ago

Ok, so the readonly modifier describes a field that acts like a property in C# with public get, private set. The easiest way to implement this would be an auto property with public get private set. Would that be OK?

DenGhostYY commented 3 months ago

From Alaska documentation about the readonly

The option READONLY limits write access for an instance variable. The assignment of a value to an instance variable declared READONLY is only permitted within the source code of methods. If the instance variable has global visibility (visibility attribute EXPORTED:), an assignment can be made within methods of the class and its subclasses. If the visibility attribute is PROTECTED:, READONLY limits the assignment to the methods of the declared class.

That is, depending on the visibility of the field, the assignment area will also change

RobertvanderHulst commented 3 months ago

DotNet does not have a different visibility for GET/SET for fields (class variables) So we will have to translate

exported:
var nNumber assignment hidden

into an auto property with a different visibility for the setter and getter

in X# Core class syntax that would become

class Example
    public property nNumber as USUAL AUTO GET HIDDEN SET
    constructor(nNumber)
        ::nNumber := nNumber
    METHOD SetNumber(nNumber)
        ::nNumber := nNumber
        RETURN SELF
end class

and that will result in a runtime error when assigning the number like in this code

o:nNumber := 10
DenGhostYY commented 3 months ago

Perfect solution. Then the same solution can be applied for the keyword readonly #1490.

Yes, this assignment o:nNumber := 10 ends in a runtime error in alaska.

nvkokkalis commented 2 months ago

Robert, this solution does not fully work as expected. There is neither a compile-time nor a runtime error.

The compiler throws no error because the member access is late-bound and the compiler is happy to translate o:nNumber := 10 to an IVarPut() call.

No runtime error is thrown either (!!!), although the assignment itself is not executed. So, the output of the sample becomes:

4
15

Also, note that since it is now a property the address-of operator won't work on it (if that's applicable). It should work when passed as a ref-argument, though.