Remove dead code and re-enable functions tests

[AzureMarker]

This code is not attached to the crate's module tree and has since been moved to chalk-recursive.

I also noticed that tests/test/functions.rs is not attached to the module tree, but I'm not sure if that is intentional.

[jackh726]

I also noticed that tests/test/functions.rs is not attached to the module tree, but I'm not sure if that is intentional.

Not intentional. Can you fix that here?

[AzureMarker]

Do you want functions.rs to be removed or re-added to the module tree?

[jackh726]

Do you want functions.rs to be removed or re-added to the module tree?

Just added to the module tree.

[AzureMarker]

The last two tests in this set are supposed to flounder, but instead return "no more solutions". Maybe the floundering behavior has purposefully changed since this test was written?

[AzureMarker]

I think something like the following should be added here

chalk/chalk-solve/src/clauses.rs

Lines 423 to 426 in 0b391f8

	TyKind::OpaqueType(opaque_ty_id, _) => {
	db.opaque_ty_data(*opaque_ty_id)
	.to_program_clauses(builder, environment);
	}

if /* trait does not appear in opaque type bound */ {
    return Err(Floundered);
}

[matthewjasper]

New output looks correct here

[jackh726]

Well, I had really hoped that the tests wouldn't fail when we readded functions.rs. Seems to have been accidently removed in f63aee5 and the failing test was moved back into that file in 039d0ba. These tests must have been broken at or after that PR. Let me think about the right solution.

[jackh726]

@Mcat12 So this ends up being a bit of a mess. See master...jackh726:functions for a diff to fix the tests. Though, when doing that, I found that there are some weirdness to discuss.
So, two options:

1. Go back to just removing the dead code and I can just merge that. Do the functions changes in another PR (you or I).
2. Apply the diff I have. We'll have to wait on some input/discussion from @nikomatsakis and/or @detrumi wrt. the opaque type bits in function_implement_fn_traits. This means we probably won't merge until next year, since holidays and such.

[AzureMarker]

I'm fine with working on the real fix for this, especially since the original change was just removing dead code (not blocking anything functional).

Edit: Hm, the tests fail on this branch (based on master) but pass on your branch (based on an older commit). I don't see any obvious issues from looking at the diff between the branches.

[jackh726]

I'm fine with working on the real fix for this, especially since the original change was just removing dead code (not blocking anything functional).

Edit: Hm, the tests fail on this branch (based on master) but pass on your branch (based on an older commit). I don't see any obvious issues from looking at the diff between the branches.

Ah oops, didn't realize I was using an outdated base. The problem is that we should also be floundering for general BoundVars too

Edit: that also explains why the recursive solver was giving different results for me

[jackh726]

This LGTM, but I essentially wrote most of this, I'd like someone else to review. @matthewjasper does this look good to you?

[matthewjasper]

@bors r+

[bors]

📌 Commit 4c2cb04 has been approved by matthewjasper

[bors]

⌛ Testing commit 4c2cb04 with merge a222811...

[bors]

☀️ Test successful - checks-actions
Approved by: matthewjasper
Pushing a222811 to master...