Use salsa tracking to link InputFile to InputIngot
#1070
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
micahscopes
force-pushed
the
salsa-track-inputs
branch
from
d6ceb63 to
68fc318
Compare
micahscopes
commented
Mar 31, 2025
| pub struct InputFile { | ||
| /// A path to the file from the ingot root directory. | ||
| #[return_ref] | ||
| #[set(__set_path_impl)] |
There was a problem hiding this comment.
If InputFile instances are loosely tied to InputIngot instances, we need to discourage people from modifying InputFile.path directly.
This could be handled by some file_rename method on InputIngot. It could also be real nice to replace InputIngot::files with a map or patricia trie to get nice cheap getters. If we're already going funnel InputFile::path modification through an InputIngot::file_rename method, maintaining that index would be better justified.
micahscopes
commented
Mar 31, 2025
Comment on lines
+119
to
+124
| // Check if the ingot already has a file with the same path | ||
| for file in ingot.files(db).iter() { | ||
| if file.path(db).as_path() == path.path(db) { | ||
| return *file; | ||
| } | ||
| } |
There was a problem hiding this comment.
This was necessary so that ingots with manually created/added InputFiles (right now just the core ingot) will have their preset InputFiles returned.
There was a problem hiding this comment.
micahscopes
force-pushed
the
salsa-track-inputs
branch
from
68fc318 to
1055f2e
Compare
micahscopes
commented
Apr 1, 2025
| #[folder = "../../library/core"] | ||
| struct Core; | ||
|
|
||
| #[salsa::tracked] |
There was a problem hiding this comment.
Disabling salsa tracking on the core ingot gets rid of the errors in CI. I can't see why these particular tests should fail as a result of reusing the core ingot:
running 17 tests
test run_name_resolution__import_alias_cycle ... ok
test run_name_resolution__conflict_variant ... ok
test run_name_resolution__import_cycle ... ok
test run_name_resolution__conflict_field ... ok
test run_name_resolution__import_missing ... ok
test run_name_resolution__import_invisible ... ok
test run_name_resolution__import_conflict ... ok
test run_name_resolution__conflict_generics ... ok
test run_name_resolution__path_shadow ... ok
test run_name_resolution__conflict ... ok
test run_name_resolution__import_unimpotable ... ok
test run_name_resolution__record_field_visibility ... ok
test run_name_resolution__path_invalid_domain ... ok
test run_name_resolution__path_missing_generics ... ok
test run_name_resolution__trait_visibility ... ok
test run_name_resolution__import_ambiguous ... FAILED
test run_name_resolution__import_ambiguous_builtin ... FAILED
failures:
---- run_name_resolution__import_ambiguous stdout ----
━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot file: fixtures/name_resolution/import_ambiguous.snap
Snapshot: import_ambiguous
Source: /home/micah/hacker-stuff-2023/fe-stuff/fe/crates/uitest:21
Input file: fixtures/name_resolution/import_ambiguous.fe
──────────────────────────────────────────────────────────────────────────
Expression: diags
──────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬─────────────────────────────────────────────────────────────
0 │-error[2-0004]: `S` is ambiguous
1 │- ┌─ import_ambiguous.fe:2:9
2 │- │
3 │- 2 │ pub use S
4 │- │ ^ `S` is ambiguous
5 │- ·
6 │-11 │ pub struct S {}
7 │- │ - candidate 1
8 │- ·
9 │-14 │ pub struct S {}
10 │- │ - candidate 2
11 │-
12 │-error[2-0004]: `S` is ambiguous
13 │- ┌─ import_ambiguous.fe:7:13
14 │- │
15 │- 7 │ pub use S
16 │- │ ^ `S` is ambiguous
17 │- ·
18 │-11 │ pub struct S {}
19 │- │ - candidate 1
20 │- ·
21 │-14 │ pub struct S {}
22 │- │ - candidate 2
────────────┴─────────────────────────────────────────────────────────────
To update snapshots run `cargo insta review`
Stopped on the first failure. Run `cargo insta test` to run all snapshots.
thread 'run_name_resolution__import_ambiguous' panicked at /home/micah/.cargo/registry/src/index.crates.io-6f17d22bba15001f/insta-1.42.2/src/runtime.rs:679:13:
snapshot assertion for 'import_ambiguous' failed in line 21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- run_name_resolution__import_ambiguous_builtin stdout ----
━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot file: fixtures/name_resolution/import_ambiguous_builtin.snap
Snapshot: import_ambiguous_builtin
Source: /home/micah/hacker-stuff-2023/fe-stuff/fe/crates/uitest:21
Input file: fixtures/name_resolution/import_ambiguous_builtin.fe
──────────────────────────────────────────────────────────────────────────
Expression: diags
──────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬─────────────────────────────────────────────────────────────
0 │-error[2-0004]: `i32` is ambiguous
0 │+error[2-0005]: `i32` can't be used as a middle segment of a path
1 1 │ ┌─ import_ambiguous_builtin.fe:3:5
2 2 │ │
3 3 │ 3 │ use i32::*
4 │- │ ^^^ `i32` is ambiguous
4 │+ │ ^^^ `i32` can't be used as a middle segment of a path
────────────┴─────────────────────────────────────────────────────────────
To update snapshots run `cargo insta review`
Stopped on the first failure. Run `cargo insta test` to run all snapshots.
thread 'run_name_resolution__import_ambiguous_builtin' panicked at /home/micah/.cargo/registry/src/index.crates.io-6f17d22bba15001f/insta-1.42.2/src/runtime.rs:679:13:
snapshot assertion for 'import_ambiguous_builtin' failed in line 21
failures:
run_name_resolution__import_ambiguous
run_name_resolution__import_ambiguous_builtin
test result: FAILED. 15 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.06s
c.c. @sbillig @g-r-a-n-t this is pretty weird
This was referenced Apr 1, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.