-
Notifications
You must be signed in to change notification settings - Fork 5
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
Ranged Integers #66
base: master
Are you sure you want to change the base?
Ranged Integers #66
Conversation
BinaryOperator::Lesser => ( | ||
(self.new_int_type(), self.new_int_type()), | ||
BOOL_CONCRETE_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.
I wonder, it's not something that should concern us too badly, but maybe the boolean comparison ops should unify(left, right)
. Maybe, maybe not.
src/to_string.rs
Outdated
ConcreteType::Named(global_ref) => { | ||
f.write_str(&self.linker_types[global_ref.id].link_info.get_full_name())?; | ||
for template_arg in global_ref.template_args.iter() { | ||
write!(f, "{}", template_arg.1.display(self.linker_types))?; |
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.
You should add the field names. Actually, there is a function that does this already: pretty_print_concrete_instance
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.
Where do I get the &LinkInfo
from? Should it be added to the display proxy?
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.
?
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.
&self.linker_types[global_ref.id].link_info
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.
Ok I have implemented that now but is there a reason pretty_print_concrete_instance
does not return a impl Display
but a String
? Thats the exact issue display proxys where meant to solve.
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.
I guess where I used it, it's for setting the name field of link_info, which is a String. But having it return impl Display would be more general.
} | ||
}; | ||
let mut template_args: FlatAlloc<ConcreteType, TemplateIDMarker> = FlatAlloc::new(); | ||
template_args.alloc(ConcreteType::new_int(out_size)); |
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.
Could be combined with ConcreteType::new_int_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.
How exactly? Would I call TypeSubstitutor::new_int_type
and unify the unknown from the template args with the expected size?
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.
I meant that new_int_type could itself use new_int. But maybe combining them reduces usability. Perhaps a rename of new_int_type to new_unknown_int would be opportune
Add test cases |
Add types to in code values / constants |
Cast module |
Fix this:
|
Well it seems that that on its own should never typecheck. Rather either the user should specify a cast, or add a |
Close #28