Allow translating locations through a sum of consecutive mappings

[maciektr]

No description provided.

[reviewable-StarkWare]

This change is 

[mkaput]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @maciektr and @orizi)

---

a discussion (no related file):
Are you able to write a test that didn't work previously but works now? I recall it was something related to expressions...

[orizi]

Reviewed all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @maciektr)

---

crates/cairo-lang-defs/src/db.rs line 663 at r2 (raw file):

                .intern(db);
                let mut code_mappings = generated.code_mappings;
                code_mappings.sort_by_key(|mapping| mapping.span.end);

this is wrong.

the order of the mappings is important - as we use them at the order they are given.

Code quote:

                let mut code_mappings = generated.code_mappings;
                code_mappings.sort_by_key(|mapping| mapping.span.end);

[maciektr]

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @mkaput and @orizi)

---

a discussion (no related file):

Previously, mkaput (Marek Kaput) wrote…

Are you able to write a test that didn't work previously but works now? I recall it was something related to expressions...

Done.

---

crates/cairo-lang-defs/src/db.rs line 663 at r2 (raw file):

Previously, orizi wrote…

this is wrong.

the order of the mappings is important - as we use them at the order they are given.

Oh, didn't realize that, sorry!

[orizi]

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @maciektr, @mkaput, and @wawel37)

---

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

}

fn translate_location(code_mapping: &[CodeMapping], span: TextSpan) -> Option<TextSpan> {

please doc - and somewhere doc the algorithm here - as i don't undersatnd it.

[orizi]

Reviewed all commit messages.
Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @maciektr, @mkaput, and @wawel37)

---

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

Previously, orizi wrote…

please doc - and somewhere doc the algorithm here - as i don't undersatnd it.

thanks - for the elaboration - are we sure we actually want to do this?
specifically - the new patch-builder basically almost never allows you to not have a containing span for all generated code - so the only thing that makes sense to me is to add some ordering to improve the lookups.

this currently seems to just hide possible issues from us actually finding them.

[mkaput]

Reviewed 2 of 5 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @maciektr and @wawel37)

[wawel37]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @maciektr)

---

crates/cairo-lang-filesystem/src/span.rs line 33 at r4 (raw file):

        self.0
    }
    pub fn from_u32(value: u32) -> Self {

There is a method here for a TextWidth that does exactly the same new_for_testing. Maybe we can just change the name of it instead of making the new one?

[mkaput]

Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @maciektr)

[mkaput]

Reviewed 4 of 4 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @maciektr)

[maciektr]

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

Previously, orizi wrote…

thanks - for the elaboration - are we sure we actually want to do this?
specifically - the new patch-builder basically almost never allows you to not have a containing span for all generated code - so the only thing that makes sense to me is to add some ordering to improve the lookups.

this currently seems to just hide possible issues from us actually finding them.

Yes, but we need this for procedural macros, rather than the compiler source plugins.

While it's true using PatchBuilder will be enough to ensure appropriate code mappings are created in the compiler, we want to be able to create code mappings even without the knowledge of real compiler AST. What we do know, is where does a single token in the returned Cairo code come from. This way, we want to keep the same behavior for compiler created code mappings, which will always translate by matching some span in code mappings list, while also using the knowledge about token origin to map errors if possible.

[orizi]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @maciektr and @wawel37)

---

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

        // Found a span that fully contains the current one - translates it.
        return containing.translate(span);
    }

can you maybe mix these in the cases where the 2nd one is found?
this seems like we are creating the extra vector for no reason in most cases.

Code quote:

    let matched = code_mapping
        .iter()
        .filter(|mapping| {
            // Omit mappings to the left or to the right of current span.
            !(mapping.span.end < span.start || mapping.span.start > span.end)
        })
        .collect::<Vec<_>>();

    // If any of the mappings fully contains the span, return the origin span of the mapping.
    if let Some(containing) = matched.iter().find(|mapping| {
        mapping.span.contains(span) && !matches!(mapping.origin, CodeOrigin::CallSite(_))
    }) {
        // Found a span that fully contains the current one - translates it.
        return containing.translate(span);
    }

[wawel37]

Reviewed 2 of 3 files at r5, 4 of 4 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @maciektr)