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

Remove dependence on specific Infos #462

Open
UnsignedByte opened this issue Sep 30, 2024 · 5 comments
Open

Remove dependence on specific Infos #462

UnsignedByte opened this issue Sep 30, 2024 · 5 comments
Labels
bug Something isn't working C: Internals Component: Compiler internals good first issue Good for newcomers

Comments

@UnsignedByte
Copy link
Collaborator

@ethanuppal has mentioned issues that arise when every info is set to empty. This is something that arises from the info_cast macro defined in info.rs:

macro_rules! info_cast {
($class:tt, $name:ident) => {
impl Info {
pub fn $name(&self) -> Option<&$class> {
match self {
Self::$class(v) => Some(v),
Self::Empty(_) => None,
_ => unreachable!(
"Incorrect info type for {}.",
stringify!($class)
),
}
}
}
impl<'a> From<&'a Info> for &'a $class {
fn from(info: &'a Info) -> Self {
if let Info::$class(v) = info {
v
} else {
unreachable!(
"Incorrect info type for {}.",
stringify!($class)
);
}
}
}
impl<'a> From<&'a Info> for Option<&'a $class> {
fn from(info: &'a Info) -> Self {
info.$name()
}
}
};
}

It defines the From trait for infos, which panics if the incorrect info type is found. This is used in certain locations primarily to generate ir::Info::Reasons for constraints (some listed below). These should theoretically just return Info::Empty if either of the parents is missing, rather than panicking, so that we could remove the dependency on info entirely throughout the IR.

We should instead implement TryFrom or instead just directly force the user to use the as_type functions that are already defined.

Non-exhaustive examples

let &ir::info::Port { bind_loc, .. } = comp.get(*info).into();
let wf = comp.add(
ir::info::Reason::misc(
"end of port access must greater than the start",
loc,
)
.into(),
);

let &ir::info::EventBind {
ev_delay_loc,
bind_loc,
} = comp.get(*info).into();
let this_ev = &comp[comp[*arg].event];
let this_delay = this_ev.delay.clone();
let &ir::info::Event {
delay_loc: ev_del_loc,
..
} = comp.get(this_ev.info).into();
let reason = comp.add(
ir::info::Reason::event_trig(
ev_delay_loc,
inv_delay.clone(),
ev_del_loc,
this_delay.clone(),
bind_loc,
)
.into(),
);

@UnsignedByte UnsignedByte added bug Something isn't working C: Internals Component: Compiler internals good first issue Good for newcomers labels Sep 30, 2024
@rachitnigam
Copy link
Member

Hm, some of these are tricky because they are not quite "info" (i.e., optional things that you are allowed to forget)...

@UnsignedByte
Copy link
Collaborator Author

What do you mean by this? I was under the assumption that the ir::Info::Reasons were used purely for pretty printing diagnostics if they occurred, and that the assumptions added could still be done with Info::Empty instead?

@rachitnigam
Copy link
Member

The two examples you quoted have info::EventBind and info::Port

@UnsignedByte
Copy link
Collaborator Author

Yes but those are just used to get bind_locs to generate the other infos, so are not really necessary either (just propogate empties or loc::unknown)

@ethanuppal
Copy link
Member

In my code I was pretty safe to just fill dummy infos with no location — that being said, I couldn't figure out how to actually get invocations working, so maybe invocations have a legitimate dependency on AST location info

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working C: Internals Component: Compiler internals good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants