-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Cranelift: ISLE recursion check #12474
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
base: main
Are you sure you want to change the base?
Changes from all commits
bdee7b9
1852038
16ae5e2
dec99be
926bd76
732f337
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,6 +80,8 @@ pub struct Decl { | |
| pub multi: bool, | ||
| /// Whether this term's constructor can fail to match. | ||
| pub partial: bool, | ||
| /// Whether this term is recursive. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/is recursive/is permitted to be recursive/, right? If you wouldn't mind, it'd be great to update the langref at the same time (
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, after reading further, I see that actually all terms in the cycle must have |
||
| pub rec: bool, | ||
| pub pos: Pos, | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ impl std::fmt::Debug for Errors { | |
| Error::TypeError { msg, .. } => format!("type error: {msg}"), | ||
| Error::UnreachableError { msg, .. } => format!("unreachable rule: {msg}"), | ||
| Error::OverlapError { msg, .. } => format!("overlap error: {msg}"), | ||
| Error::RecursionError { msg, .. } => format!("recursion error: {msg}"), | ||
| Error::ShadowedError { .. } => { | ||
| "more general higher-priority rule shadows other rules".to_string() | ||
| } | ||
|
|
@@ -33,7 +34,8 @@ impl std::fmt::Debug for Errors { | |
|
|
||
| Error::ParseError { span, .. } | ||
| | Error::TypeError { span, .. } | ||
| | Error::UnreachableError { span, .. } => { | ||
| | Error::UnreachableError { span, .. } | ||
| | Error::RecursionError { span, .. } => { | ||
| vec![Label::primary(span.from.file, span)] | ||
| } | ||
|
|
||
|
|
@@ -127,6 +129,15 @@ pub enum Error { | |
| rules: Vec<Span>, | ||
| }, | ||
|
|
||
| /// Recurive rules error. Term is recursive without explicit opt-in, or vice versa. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "or vice versa" implies that if a term is marked |
||
| RecursionError { | ||
| /// The error message. | ||
| msg: String, | ||
|
|
||
| /// The location of the term declaration. | ||
| span: Span, | ||
| }, | ||
|
|
||
| /// The rules can never match because another rule will always match first. | ||
| ShadowedError { | ||
| /// The locations of the unmatchable rules. | ||
|
|
||
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.
In each case where we have a
rec, could we have a comment describing why it's bounded?In particular I'm thinking these should be modeled after something like Rust safety comments -- so e.g.
// Recursion: may recurse once to reuse implementation for F32 in the case of F16or// Recursion: bounded by explicit depth parameter 'depth'or// Recursion: each iteration of count_the_bits_in_isle removes one bit from value and completes when zeroor something like that.