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

add some wf checks for fn types #181

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

add some wf checks for fn types #181

wants to merge 3 commits into from

Conversation

shua
Copy link
Contributor

@shua shua commented Jul 2, 2024

This change adds a syntax for specifying FnDefs and FnPtrs as type parameters, fn name<T, U, ...> and fn(T, U,...) -> V respectively. The fn prefix is used to disambiguate between adt identifiers. In addition, some relevant well-formed cases have been added.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Basically looks good — are there any tests for function WF though? Also, the debug impl didn't quite look like what I expected, which is to output fn name<...> and fn(...). I think it will just emit name<...>and (...)?

@@ -20,13 +20,13 @@ fn decls() -> Decls {
fn direct_cycle() {
test_prove(decls(), term("exists<ty A> {} => {A = Vec<A>}")).assert_err(
expect![[r#"
judgment `prove_wc_list { goal: {?ty_0 = Vec<?ty_0>}, assumptions: {}, env: Env { variables: [?ty_0], bias: Soundness }, decls: decls(222, [trait Foo <ty> ], [impl <ty> Foo(Vec<^ty0_0>)], [], [], [], [], {}, {}) }` failed at the following rule(s):
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably cleanup the Env Debug impl to e.g. suppress empty lists...

Copy link
Contributor Author

@shua shua Jul 11, 2024

Choose a reason for hiding this comment

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

#187 smaller decls output

crates/formality-types/src/grammar/ty/debug_impls.rs Outdated Show resolved Hide resolved
Comment on lines 47 to 56
fn wf() {
crate::assert_ok!(
[
crate Foo {
fn foo<ty T>() -> T { trusted }
fn bar(fn foo<u32>, fn(u32) -> (), fn(bool)) -> () { trusted }
}
]

expect_test::expect![[r#"
judgment `prove_wc_list { goal: {@ wf(&!lt_0 !ty_1)}, assumptions: {}, env: Env { variables: [!lt_0, !ty_1], bias: Soundness }, decls: decls(222, [], [], [], [], [], [], {}, {}) }` failed at the following rule(s):
the rule "some" failed at step #0 (src/file.rs:LL:CC) because
judgment `prove_wc { goal: @ wf(&!lt_0 !ty_1), assumptions: {}, env: Env { variables: [!lt_0, !ty_1], bias: Soundness }, decls: decls(222, [], [], [], [], [], [], {}, {}) }` failed at the following rule(s):
the rule "parameter well formed" failed at step #0 (src/file.rs:LL:CC) because
judgment had no applicable rules: `prove_wf { goal: &!lt_0 !ty_1, assumptions: {}, env: Env { variables: [!lt_0, !ty_1], bias: Soundness }, decls: decls(222, [], [], [], [], [], [], {}, {}) }`"#]]
expect_test::expect!["()"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nikomatsakis this is a test for fn well-formed. I can also add some tests for catching poorly formed fns.

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

mind blocking this PR on #187 (comment) reviewing it with most of the changes being an added [] to decls isn't great

@shua shua requested review from lcnr and nikomatsakis July 20, 2024 19:19
@shua
Copy link
Contributor Author

shua commented Oct 17, 2024

good time for re-review @lcnr or @nikomatsakis ?

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Oct 17, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants