-
Notifications
You must be signed in to change notification settings - Fork 183
Intern Vec<ProgramClause<I>> #370
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
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.
Thanks! A few nits.
chalk-ir/src/lib.rs
Outdated
|
||
#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord, HasInterner)] | ||
pub struct ProgramClause<I: Interner> { | ||
clause: I::InternedProgramClause, |
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.
Nit: I think we we call this interned
in all the other structs
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.
We call it interned
except in Goal
and Goals
IIRC. I think those should be renamed and everything should be interned
.
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.
It looks like @tirr-c fixed the new cases in this PR, we can create an issue for the others
pub fn data(&self, interner: &I) -> &ProgramClauseData<I> { | ||
interner.program_clause_data(&self.clause) | ||
} | ||
} |
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.
Also I think we should add an interned(&self) -> &I::InternedProgramClause
accessor
Looks like the tests need to be updated, too |
@nikomatsakis Done, pushed changes! |
This PR interns
ProgramClause
andVec<ProgramClause>
, and makes changes accordingly.