Check that built-in callable types validate their output type is `Sized` (in new solver) #107867

compiler-errors · 2023-02-10T03:42:41Z

Working on parity with old solver. Putting this up for consideration, it's not really needed or anything just yet. Maybe it's better to approach this from another direction (like always checking the item bounds when calling consider_assumption? we may need that for coinduction to be sound though?)

This basically implements #100096 for the new solver.

rustbot · 2023-02-10T03:42:47Z

r? @eholk

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-02-10T03:42:51Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

lcnr

Am in favor of landing this PR.

some related thoughts wrt naming n stuff: I am actually not too happy about the name consider_assumption given how it is used.

If we have a builtin impl we're not really considering an assumption as it's something where we know that it holds, we're simply looking at an impl header with no nested goals.

If we were to change consider_assumption to also take a list of nested obligations it would be pretty much how candidates in chalk (and probably a-mir-formality) work?

So maybe a name like fn consider_normal_candidate (with a comment explaining what normal means might be better? 🤔 idk

compiler/rustc_trait_selection/src/solve/project_goals.rs

compiler/rustc_trait_selection/src/solve/trait_goals.rs

compiler-errors · 2023-02-16T03:07:15Z

I renamed it to consider_hypothesis and moved some stuff around. Not super attached to anything, so please let me know if you think things should be renamed or moved.

@rustbot ready

lcnr · 2023-02-17T08:53:03Z

compiler/rustc_trait_selection/src/solve/assembly.rs

        ecx: &mut EvalCtxt<'_, 'tcx>,
        goal: Goal<'tcx, Self>,
-        impl_def_id: DefId,
+        assumption: ty::Predicate<'tcx>,
+        requirements: Vec<Goal<'tcx, ty::Predicate<'tcx>>>,


impl IntoIterator<Item = Goal> maybe. We have a bunch of fixed len requirements which can just be [goal] 🤔

lcnr · 2023-02-17T09:06:20Z

compiler/rustc_trait_selection/src/solve/assembly.rs

+    // Use a hypothesis, which consists of a "assumption" and some "requirements",
+    // to satisfy a goal. If the requirements hold, then attempt to satisfy our
+    // goal by equating it with the assumption.
+    fn consider_hypothesis(


not too happy with hypothesis https://www.merriam-webster.com/dictionary/hypothesis

A hypothesis is an assumption, an idea that is proposed for the sake of argument so that it can be tested to see if it might be true.

In the scientific method, the hypothesis is constructed before any applicable research has been done, apart from a basic background review. You ask a question, read up on what has been studied before, and then form a hypothesis.

we know that the requirements -> assumption is correct. Looking at chalk, they use clause for this. I don't think we should do that in rustc because we already have a Clause which (at least for now) is something slightly different.

Other ideas:

consider_(logical)_statement (meh)

consider_implication (this will be how implications in the param env will be implemented (if and once we get them))

consider_implied_clause (changing assumption to clause: ty::Clause, will actually re)

I personally kinda like consider_implied_clause 🤔 It is a clause which is implied by some requirements. But yeah, if you don't like consider_implied_clause we can just go with whatever.

r=me after nit

Yeah, consider_implied_clause is a totally fine name!

…ed` (in new solver)

compiler-errors · 2023-02-18T20:33:45Z

@bors r=lcnr

bors · 2023-02-18T20:33:47Z

📌 Commit 6402c98 has been approved by lcnr

It is now in the queue for this repository.

bors · 2023-02-19T02:42:13Z

⌛ Testing commit 6402c98 with merge fcdbd1c...

bors · 2023-02-19T05:34:58Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing fcdbd1c to master...

rust-timer · 2023-02-19T06:41:15Z

Finished benchmarking commit (fcdbd1c): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.0%	[1.0%, 1.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

rustbot assigned eholk Feb 10, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Feb 10, 2023

compiler-errors force-pushed the new-solver-fn-trait-safety branch from 03cbc19 to a7c3415 Compare February 10, 2023 04:11

lcnr reviewed Feb 10, 2023

View reviewed changes

compiler/rustc_trait_selection/src/solve/project_goals.rs Outdated Show resolved Hide resolved

compiler/rustc_trait_selection/src/solve/trait_goals.rs Outdated Show resolved Hide resolved

compiler-errors assigned lcnr and unassigned eholk Feb 15, 2023

lcnr added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 15, 2023

compiler-errors force-pushed the new-solver-fn-trait-safety branch 2 times, most recently from 454d4a6 to 189e26f Compare February 16, 2023 03:06

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 16, 2023

lcnr reviewed Feb 17, 2023

View reviewed changes

compiler-errors added 2 commits February 18, 2023 19:32

Check that built-in callable types validate their output type is `Siz…

82b5205

…ed` (in new solver)

Add consider_implied_clause

6402c98

compiler-errors force-pushed the new-solver-fn-trait-safety branch from 189e26f to 6402c98 Compare February 18, 2023 19:45

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 18, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 19, 2023

bors merged commit fcdbd1c into rust-lang:master Feb 19, 2023

rustbot added this to the 1.69.0 milestone Feb 19, 2023

compiler-errors deleted the new-solver-fn-trait-safety branch August 11, 2023 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check that built-in callable types validate their output type is `Sized` (in new solver) #107867

Check that built-in callable types validate their output type is `Sized` (in new solver) #107867

compiler-errors commented Feb 10, 2023 •

edited

Loading

rustbot commented Feb 10, 2023

rustbot commented Feb 10, 2023

lcnr left a comment •

edited

Loading

compiler-errors commented Feb 16, 2023

lcnr Feb 17, 2023

lcnr Feb 17, 2023

compiler-errors Feb 18, 2023

compiler-errors commented Feb 18, 2023

bors commented Feb 18, 2023

bors commented Feb 19, 2023

bors commented Feb 19, 2023

rust-timer commented Feb 19, 2023

Check that built-in callable types validate their output type is Sized (in new solver) #107867

Check that built-in callable types validate their output type is Sized (in new solver) #107867

Conversation

compiler-errors commented Feb 10, 2023 • edited Loading

rustbot commented Feb 10, 2023

rustbot commented Feb 10, 2023

lcnr left a comment • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Feb 16, 2023

lcnr Feb 17, 2023

Choose a reason for hiding this comment

lcnr Feb 17, 2023

Choose a reason for hiding this comment

compiler-errors Feb 18, 2023

Choose a reason for hiding this comment

compiler-errors commented Feb 18, 2023

bors commented Feb 18, 2023

bors commented Feb 19, 2023

bors commented Feb 19, 2023

rust-timer commented Feb 19, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Check that built-in callable types validate their output type is `Sized` (in new solver) #107867

Check that built-in callable types validate their output type is `Sized` (in new solver) #107867

compiler-errors commented Feb 10, 2023 •

edited

Loading

lcnr left a comment •

edited

Loading