-
Notifications
You must be signed in to change notification settings - Fork 550
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
added validation for generic params of impl functions #6934
added validation for generic params of impl functions #6934
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 4 files at r1, all commit messages.
Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)
crates/cairo-lang-semantic/src/items/imp.rs
line 3169 at r1 (raw file):
} match (generic_param, trait_generic_param) { (GenericParam::Type(_), GenericParam::Type(_)) => {}
we should probably validate names here as well.
(same as we do in non-generic fn params)
Code quote:
(GenericParam::Type(_), GenericParam::Type(_)) => {}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @orizi)
crates/cairo-lang-semantic/src/items/imp.rs
line 3169 at r1 (raw file):
Previously, orizi wrote…
we should probably validate names here as well.
(same as we do in non-generic fn params)
isn't it the validation I added in 5 lines above or you mean something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 4 files at r1.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)
crates/cairo-lang-semantic/src/items/imp.rs
line 3169 at r1 (raw file):
Previously, TomerStarkware wrote…
isn't it the validation I added in 5 lines above or you mean something else?
no - i just missed it.
crates/cairo-lang-semantic/src/items/tests/trait
line 1302 at r1 (raw file):
//! > ========================================================================== //! > cycle with anonymous impl with impl alias instead of trait.
this test are deleted?
4c59eb6
to
26b9ef9
Compare
26b9ef9
to
4fb4307
Compare
There was a problem hiding this 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 4 files reviewed, 1 unresolved discussion (waiting on @orizi)
crates/cairo-lang-semantic/src/items/tests/trait
line 1302 at r1 (raw file):
Previously, orizi wrote…
this test are deleted?
reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware)
No description provided.