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.

Sign up for GitHub

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

Jump to bottom

Generic type for Trait functions #1553

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Generic type for Trait functions #1553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b1c16a
Grammar fixed
gzanitti Jul 9, 2024
eefacf0
Ambiguity in grammar fixed: Change of function separator in traits fr…
gzanitti Jul 9, 2024
2cc4ea6
Missed test fixed
gzanitti Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion asm-to-pil/src/vm_to_constrained.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1289,7 +1289,7 @@ machine Main {
}

trait ArraySum<T> {
array_sum: T[4 + 1] -> DoubleOpt<T>,
array_sum: T[4 + 1] -> DoubleOpt<T>;
}
}

Expand Down
2 changes: 1 addition & 1 deletion ast/src/parsed/display.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ impl<E: Display> Display for TraitDeclaration<E> {
name = self.name,
type_vars = self.type_vars.iter().format(", "),
functions = indent(
self.functions.iter().map(|m| format!("{m},\n")).format(""),
self.functions.iter().map(|m| format!("{m};\n")).format(""),
1
)
)
Expand Down
14 changes: 7 additions & 7 deletions parser/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,12 +485,12 @@ namespace N(2);
fn parse_trait() {
let input = r#"
trait Add<T> {
add: T, T -> T,
add: T, T -> T;
}"#;

let expected = r#"
trait Add<T> {
add: T, T -> T,
add: T, T -> T;
}"#;

let printed = format!("{}", parse(Some("input"), input).unwrap_err_to_stderr());
Expand All @@ -501,12 +501,12 @@ namespace N(2);
fn parse_trait_multi_params() {
let input = r#"
trait Add<T, Q> {
add: T, T -> Q,
add: T, T -> Q;
}"#;

let expected = r#"
trait Add<T, Q> {
add: T, T -> Q,
add: T, T -> Q;
}"#;

let printed = format!("{}", parse(Some("input"), input).unwrap_err_to_stderr());
Expand All @@ -518,7 +518,7 @@ namespace N(2);
fn parse_trait_no_type_vars() {
let input = r#"
trait Add {
add: int, int -> int,
add: int, int -> int;
}"#;

let _ = format!("{}", parse(Some("input"), input).unwrap_err_to_stderr());
Expand All @@ -528,12 +528,12 @@ namespace N(2);
fn parse_trait_multi_params2() {
let input = r#"
trait Iterator<S, I> {
next: S -> (S, Option<I>),
next: S -> (S, Option<I>);
}"#;

let expected = r#"
trait Iterator<S, I> {
next: S -> (S, Option<I>),
next: S -> (S, Option<I>);
}"#;

let printed = format!("{}", parse(Some("input"), input).unwrap_err_to_stderr());
Expand Down
4 changes: 2 additions & 2 deletions parser/src/powdr.lalrpop
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,11 +691,11 @@ TraitVars: Vec<String> = {

TraitFunctions: Vec<TraitFunction<Expression>> = {
=> vec![],
<mut list:( <TraitFunction> "," )*> <end:TraitFunction> "," => { list.push(end); list }
<mut list:( <TraitFunction> ";" )*> <end:TraitFunction> ";" => { list.push(end); list }
}

TraitFunction: TraitFunction<Expression> = {
<name:Identifier> ":" <params:TypeTermList> "->" <value:TypeTermBox> => TraitFunction { name, ty: Type::Function(FunctionType{params, value}) }
<name:Identifier> ":" <ty:Type> => TraitFunction { name, ty }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know where the parsing ambiguity comes from?

I think we could change this to TypeTerm here and fix it, but then we would need to use parentheses for functions which is not ideal.

Would it be an option to have to lines here, one for an explicit function as before and one for TypeTerm?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, it's not too bad to use ;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok I think it's because a: x, y, z -> w would need too much look-ahead to find out the meaning of the , after the x. Although it's really not too much look-ahead, it's just a single token...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok I think it's because a: x, y, z -> w would need too much look-ahead to find out the meaning of the , after the x. Although it's really not too much look-ahead, it's just a single token...

Yes, exactly, by using commas at the end of functions, the parser cannot differentiate when it's another type and when it's the end of the function.

Would it be an option to have to lines here, one for an explicit function as before and one for TypeTerm?

Not really, because it would be equivalent to simply "opening" Type in its two possible variants and replacing them.

On the other hand, it's not too bad to use ;

And it's like in Rust 🥇 😝

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean rust traits use fn functions.

I would really like traits to be similar to structs, and I think the two alternatives would work: Note that the second is not Type but TypeTerm, so if you want to use that to represent a function type you would have to put it in parentheses. But you could still use

trait X<T> {
  some_const: T,
  some_fun: T -> T,
}

}


Expand Down
10 changes: 5 additions & 5 deletions pil-analyzer/tests/parse_display.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,12 +718,12 @@ namespace T(8);
#[test]
fn trait_def() {
let input = "trait Add<T, Q> {
add: T, T -> Q,
add: T, T -> Q;
}
";

let expected = " trait Add<T, Q> {
add: T, T -> Q,
add: T, T -> Q;
}
";

Expand All @@ -734,12 +734,12 @@ fn trait_def() {
#[test]
fn array_type_trait() {
let input = "trait ArraySum<T> {
array_sum: T[4 + 1] -> T,
array_sum: T[4 + 1] -> T;
}
";

let expected = " trait ArraySum<T> {
array_sum: T[5] -> T,
array_sum: T[5] -> T;
}
";

Expand All @@ -751,7 +751,7 @@ fn array_type_trait() {
#[should_panic = "Duplicate symbol definition: Add"]
fn trait_enum_collisions() {
let input = "trait Add<T, Q> {
add: T, T -> Q,
add: T, T -> Q;
}
enum Add {
X
Expand Down
2 changes: 1 addition & 1 deletion test_data/asm/trait_parsing.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ mod types {
}

trait ArraySum<T> {
array_sum: T[4 + 1] -> DoubleOpt<T>,
array_sum: T[4 + 1] -> DoubleOpt<T>;
}
}

Expand Down
Loading