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

Cache deref chain #7213

Merged
merged 1 commit into from
Feb 10, 2025
Merged

Cache deref chain #7213

merged 1 commit into from
Feb 10, 2025

Conversation

ilyalesokhin-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@ilyalesokhin-starkware ilyalesokhin-starkware force-pushed the ilya/deref_chain2 branch 2 times, most recently from e3d0be0 to 70bf70a Compare February 5, 2025 12:20
@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as draft February 5, 2025 12:21
@ilyalesokhin-starkware
Copy link
Contributor Author

crates/cairo-lang-semantic/src/resolve/mod.rs line 120 at r1 (raw file):

    /// A map from member names to their semantic representation and the number of deref operations
    /// needed to access them.
    pub members: OrderedHashMap<SmolStr, (Member, usize)>,

not sure if this should be move to the deref_chain query result.

Code quote:

   pub members: OrderedHashMap<SmolStr, (Member, usize)>,

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 5 files at r1, 1 of 2 files at r2, all commit messages.
Reviewable status: 4 of 5 files reviewed, 6 unresolved discussions (waiting on @gilbens-starkware and @ilyalesokhin-starkware)


crates/cairo-lang-semantic/src/items/imp.rs line 772 at r2 (raw file):

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct DerefChain {

doc


crates/cairo-lang-semantic/src/items/imp.rs line 777 at r2 (raw file):

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct DerefInfo {

doc


crates/cairo-lang-semantic/src/items/imp.rs line 783 at r2 (raw file):

}

pub fn deref_chain_cycle(

give a pointer back to the query as a doc.


crates/cairo-lang-semantic/src/items/imp.rs line 793 at r2 (raw file):

}

pub fn deref_chain(db: &dyn SemanticGroup, ty: TypeId, try_deref_mut: bool) -> Maybe<DerefChain> {

give a pointer back to the query as a doc.


crates/cairo-lang-semantic/src/items/imp.rs line 820 at r2 (raw file):

}

pub fn get_deref_func_and_target(

doc and remove pub

Copy link
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 2 of 5 files reviewed, 6 unresolved discussions (waiting on @gilbens-starkware and @orizi)


crates/cairo-lang-semantic/src/items/imp.rs line 772 at r2 (raw file):

Previously, orizi wrote…

doc

Done.


crates/cairo-lang-semantic/src/items/imp.rs line 777 at r2 (raw file):

Previously, orizi wrote…

doc

Done.


crates/cairo-lang-semantic/src/items/imp.rs line 783 at r2 (raw file):

Previously, orizi wrote…

give a pointer back to the query as a doc.

Done.


crates/cairo-lang-semantic/src/items/imp.rs line 793 at r2 (raw file):

Previously, orizi wrote…

give a pointer back to the query as a doc.

Done.

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gilbens-starkware and @ilyalesokhin-starkware)


crates/cairo-lang-semantic/src/items/imp.rs line 780 at r3 (raw file):

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct DerefInfo {
    /// deref function

Suggestion:

    /// The concrete `Deref::deref` function

@ilyalesokhin-starkware ilyalesokhin-starkware force-pushed the ilya/deref_chain2 branch 2 times, most recently from 11c6d93 to 06193eb Compare February 9, 2025 11:25
@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as ready for review February 9, 2025 11:25
@ilyalesokhin-starkware ilyalesokhin-starkware force-pushed the ilya/deref_chain2 branch 2 times, most recently from 2c85ad5 to 3bd82be Compare February 9, 2025 12:27
Copy link
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 3 of 6 files reviewed, 3 unresolved discussions (waiting on @gilbens-starkware and @orizi)


crates/cairo-lang-semantic/src/items/imp.rs line 820 at r2 (raw file):

Previously, orizi wrote…

doc and remove pub

Done.


crates/cairo-lang-semantic/src/items/imp.rs line 780 at r3 (raw file):

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct DerefInfo {
    /// deref function

Done.

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 3 files at r4, 2 of 2 files at r5.
Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware)


before.after line 0 at r5 (raw file):
what us this file?

Copy link
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)


before.after line at r5 (raw file):

Previously, orizi wrote…

what us this file?

Done.

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)

@ilyalesokhin-starkware ilyalesokhin-starkware added this pull request to the merge queue Feb 10, 2025
Merged via the queue into main with commit 8e6a25e Feb 10, 2025
48 checks passed
@ilyalesokhin-starkware ilyalesokhin-starkware deleted the ilya/deref_chain2 branch February 10, 2025 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants