hylo-lang / hylo

The Hylo programming language
https://www.hylo-lang.org
Apache License 2.0
1.16k stars 55 forks source link

Synthesized SourceFile creation doesn't respect value semantics #1489

Open tothambrus11 opened 1 month ago

tothambrus11 commented 1 month ago

When creating two SourceFile objects with the same file name, the second one does not get an independent storage from the first one. This was confusing because the next time we initialize a source file, it might have a completely different content at the same file name / url as the old one.

I encountered an issue while writing tests for something that involved creating source files with specific names, and I reduced the problem to the following:

  func testUsingSourceFileWithSameName() {
    let source1 = SourceFile(synthesizedText: "type A{}", named: "example.hylo")
    XCTAssertEqual(source1.text, "type A{}")

    let source2 = SourceFile(synthesizedText: "type B{}", named: "example.hylo")
    XCTAssertEqual(source2.text, "type B{}") // XCTAssertEqual failed: ("type A{}") is not equal to ("type B{}") - 
  }

The problem persists even if we are working within different test cases, which is particularly spooky and breaks value semantics.

  func test1UsingSourceFileWithSameName() {
    let source = SourceFile(synthesizedText: "type A{}", named: "example.hylo")
    XCTAssertEqual(source.text, "type A{}")
  }
  func test2UsingSourceFileWithSameName() {
    let source = SourceFile(synthesizedText: "type B{}", named: "example.hylo")
    XCTAssertEqual(source.text, "type B{}") // XCTAssertEqual failed: ("type A{}") is not equal to ("type B{}") - 
  }

What were the design decisions behind creating a shared storage of source files? Is this a bug or intended behaviour? Can we maybe make this issue less likely to happen, e.g. only making a shared storage when the contents of the two source files equal?

kyouko-taiga commented 1 month ago

It is intended behavior. The purpose of the shared storage is to avoid duplicating source contents while deserializing an AST loaded from disk.

Clearly the name of the file should be unique and your code is not upholding this requirement. There's no reason to give explicit names to synthetic files, though. If you look at the constructor you'll see that the default argument is a random UUID.

dabrahams commented 1 month ago

@tothambrus11 unless you have some objection we haven't considere, please close the issue. Thanks.