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.

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

Object expression should not inline variable in getter in Release mode #3779

Open
MangelMaxime opened this issue Mar 9, 2024 · 4 comments
Open
Labels

Comments

@MangelMaxime
Copy link
Member

Description

[<AbstractClass>]
type Reader() =
    abstract Warnings: ResizeArray<string> with get

let reader =
    let warnins = ResizeArray<string>()
    { new Reader() with
        member __.Warnings = warnins
    }

reader.Warnings.Add("Warning 1")
reader.Warnings.Add("Warning 2")

printfn "Count: %A" reader.Warnings.Count

In watch mode:

export const reader = (() => {
    const warnins = [];
    return new (class extends Reader {
        constructor() {
            super();
        }
        "Glutinum.Web.Main.Reader.get_Warnings"() {
            return warnins;
        }
    }
    )();
})();

In build mode:

export const reader = new (class extends Reader {
    constructor() {
        super();
    }
    "Glutinum.Web.Main.Reader.get_Warnings"() {
        return [];
    }
}
)();

This cause a problem because, now the getter always reset the value of warnings in build mode.

Meaning that in Release mode we get 0 instead of 2.

Expected and actual results

Should returns 2 in Release mode too.

Related information

  • Fable version: 4.14.0
  • Operating system: OSX
MangelMaxime added a commit to glutinum-org/cli that referenced this issue Mar 9, 2024
@ncave
Copy link
Collaborator

ncave commented Mar 9, 2024

@MangelMaxime Perhaps I'm doing something wrong, but I cannot reproduce with v4.14, with or without --watch.

import { class_type } from "../fable_modules/fable-library-js.4.14.0/Reflection.js";
import { format } from "../fable_modules/fable-library-js.4.14.0/String.js";

export class Reader {
    constructor() {
    }
}

export function Reader_$reflection() {
    return class_type("TestApp.Reader", void 0, Reader);
}

export function Reader_$ctor() {
    return new Reader();
}

export const reader = (() => {
    let warnings = [];
    return new (class extends Reader {
        constructor() {
            super();
        }
        "TestApp.Reader.get_Warnings"() {
            return warnings;
        }
    }
    )();
})();

Can you post your full build command line?

@MangelMaxime
Copy link
Member Author

@ncave Here are the steps I used to test it out:

  1. Create test.fsx with the following content

    [<AbstractClass>]
    type Reader() =
        abstract Warnings: ResizeArray<string> with get
    
    let reader =
        let warnins = ResizeArray<string>()
        { new Reader() with
            member __.Warnings = warnins
        }
    
    reader.Warnings.Add("Warning 1")
    reader.Warnings.Add("Warning 2")
    
    printfn "Count: %A" reader.Warnings.Count
  2. Run dotnet fable test.fsx, it should generates:

import { class_type } from "./fable_modules/fable-library-js.4.14.0/Reflection.js";
import { printf, toConsole } from "./fable_modules/fable-library-js.4.14.0/String.js";

export class Reader {
    constructor() {
    }
}

export function Reader_$reflection() {
    return class_type("Test.Reader", void 0, Reader);
}

export function Reader_$ctor() {
    return new Reader();
}

export const reader = new (class extends Reader {
    constructor() {
        super();
    }
    "Test.Reader.get_Warnings"() {
        return [];
    }
}
)();

void (reader["Test.Reader.get_Warnings"]().push("Warning 1"));

void (reader["Test.Reader.get_Warnings"]().push("Warning 2"));

(function () {
    const arg = reader["Test.Reader.get_Warnings"]().length | 0;
    toConsole(printf("Count: %A"))(arg);
})();

The problematic lines are:

    "Test.Reader.get_Warnings"() {
        return [];
    }

If you run dotnet fable -c Debug now you should get:

import { class_type } from "./fable_modules/fable-library-js.4.14.0/Reflection.js";
import { printf, toConsole } from "./fable_modules/fable-library-js.4.14.0/String.js";

export class Reader {
    constructor() {
    }
}

export function Reader_$reflection() {
    return class_type("Test.Reader", void 0, Reader);
}

export function Reader_$ctor() {
    return new Reader();
}

export const reader = (() => {
    const warnins = [];
    return new (class extends Reader {
        constructor() {
            super();
        }
        "Test.Reader.get_Warnings"() {
            return warnins;
        }
    }
    )();
})();

void (reader["Test.Reader.get_Warnings"]().push("Warning 1"));

void (reader["Test.Reader.get_Warnings"]().push("Warning 2"));

(function () {
    const arg = reader["Test.Reader.get_Warnings"]().length | 0;
    toConsole(printf("Count: %A"))(arg);
})();

Which is "correct".

I tested it on OSX and Linux (dev container) with Fable 4.14 instead from NuGet for both of them.

@ncave
Copy link
Collaborator

ncave commented Mar 11, 2024

@MangelMaxime Probably different default configurations when using a project file vs a script.

In theory an array is mutable so it shouldn't be beta reduced (like other mutables or side effects), we have to see if it's the F# compiler optimizing it, or Fable.

@MangelMaxime
Copy link
Member Author

@ncave Here I used fsx because it was easier for the reproduction but in my case the problem happens also when using a project file.

Here is a reproduction project:

https://github.com/MangelMaxime/fable-reproduction-issue-3779

If you run dotnet fable you will see that the array has been inlined. But if you run dotnet build -c Debug it is not.

Steps used to setup that repository
  1. dotnet new console -lang F#
  2. dotnet new tool-manifest
  3. dotnet tool install fable
  4. Change the content of Program.fs to the reproduction code (see comments above)
  5. Run dotnet fable and dotnet fable -c Debug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants