Added external files.

[orizi]

Stack:

• Added more proper code mappings in starknet plugins. #6227
• Moved defs plugins files to be based on external files. #6225
• Made calculating following files recursive. #6224
• Added external files. #6223 ⬅

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

---

This change is 

[gilbens-starkware]

Reviewed 16 of 16 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Arcticae, @Draggu, @mkaput, and @piotmag769)

[mkaput]

Reviewed 16 of 16 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)

---

crates/cairo-lang-filesystem/src/ids.rs line 65 at r1 (raw file):

    OnDisk(PathBuf),
    Virtual(VirtualFile),
    External(u32),

please add:

struct ExternalFileId(u32)

to hide u32 as implementation detail

[orizi]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, @mkaput, and @piotmag769)

---

crates/cairo-lang-filesystem/src/ids.rs line 65 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

please add:

struct ExternalFileId(u32)

to hide u32 as implementation detail

this becomes a salsa::InternId in a followup PR - so not really sensible.

[mkaput]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)

---

crates/cairo-lang-filesystem/src/ids.rs line 65 at r1 (raw file):

Previously, orizi wrote…

this becomes a salsa::InternId in a followup PR - so not really sensible.

salsa::InternId is also an implementation-detail-kind type just like u32, isn't it?

[mkaput]

Reviewed 1 of 16 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)

[mkaput]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)

---

crates/cairo-lang-filesystem/src/db.rs line 111 at r1 (raw file):

/// A trait for defining files external to the `filesystem` crate.
pub trait ExternalFiles {

why couldn't this be just a new query group?

[orizi]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, and @piotmag769)

---

crates/cairo-lang-filesystem/src/db.rs line 111 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

why couldn't this be just a new query group?

how would that work?
the idea is using it by a QueryGroup that already depends on filesystem - so that would be a cyclic dependency.

---

crates/cairo-lang-filesystem/src/ids.rs line 65 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

salsa::InternId is also an implementation-detail-kind type just like u32, isn't it?

but that wrap is meaningless.

the entire idea is that it is an external implementation using another query - the wrapper type would not solve anything.

[mkaput]

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)

---

crates/cairo-lang-filesystem/src/db.rs line 111 at r1 (raw file):

Previously, orizi wrote…

how would that work?
the idea is using it by a QueryGroup that already depends on filesystem - so that would be a cyclic dependency.

closing this thread, I'll open new one with my recent thoughts on this

---

crates/cairo-lang-filesystem/src/ids.rs line 65 at r1 (raw file):

Previously, orizi wrote…

but that wrap is meaningless.

the entire idea is that it is an external implementation using another query - the wrapper type would not solve anything.

yeah, you're right it's meaningless in this context. resolving this thread but I'll post my thoughts in another one

---

crates/cairo-lang-filesystem/src/ids.rs line 66 at r2 (raw file):

    Virtual(VirtualFile),
    External(u32),
}

The more I think and try to reason about this, the more I believe this is an unnecessary abstraction. Why can't we just put plugin generated file ID here directly? This should reduce much of the indirections that are happening here, and thus make the code easier to reason about + I really don't believe we'll need another file generation method anytime soon.

Suggestion:

pub enum FileLongId {
    OnDisk(PathBuf),
    Virtual(VirtualFile),
    PluginGenerated(PluginGeneratedFileId),
}

[orizi]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, @mkaput, and @piotmag769)

---

crates/cairo-lang-filesystem/src/ids.rs line 66 at r2 (raw file):

Previously, mkaput (Marek Kaput) wrote…

The more I think and try to reason about this, the more I believe this is an unnecessary abstraction. Why can't we just put plugin generated file ID here directly? This should reduce much of the indirections that are happening here, and thus make the code easier to reason about + I really don't believe we'll need another file generation method anytime soon.

this is a circular dependency - the entire idea is to break it.

[mkaput]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Arcticae, @Draggu, and @piotmag769)