Skip to content

opportunistically evaluate constants while populating mir::Body::required_consts #71802

Open
@oli-obk

Description

@oli-obk
Contributor

We can evaluate all constants that do not depend on generic parameters in

if let ConstKind::Unevaluated(_, _, _) = const_kind {

We make that a MutVisitor that evaluates all constants that it can and replaces them in-place and only adds those that it couldn't evaluate to the required_const list.

This way we never have to evaluate another constant in the entire MIR optimization pipeline, because we know none can be evaluated (modulo those that we already evaluate for merging required_consts during inlining.

cc @rust-lang/wg-mir-opt I think I like having the above invariant. It will make working with constants in MIR much simpler and make any kind of error handling that would have to be implemented unified into a single location.

Activity

added
A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)
A-mir-optArea: MIR optimizations
on May 2, 2020
added
C-enhancementCategory: An issue proposing an enhancement or a PR with one.
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
on May 2, 2020
RalfJung

RalfJung commented on May 2, 2020

@RalfJung
Member

and make any kind of error handling that would have to be implemented unified into a single location.

Ideally that would actually dispatch through the const_eval error handling to have it all centralized there...

Same goes for the const error reporting that codegen is doing, FWIW.

Which makes me wonder, can we move this impl block to librustc_mir/const_eval/error.rs or so? This is solely CTFE error logic, so logically part of the "CTFE <-> rustc glue" that we otherwise have in librustc_mir::const_eval.

oli-obk

oli-obk commented on May 2, 2020

@oli-obk
ContributorAuthor

hmm right, I think nothing from librustc uses this anymore. We can't move the impl block, but we can make the functions free functions.

added
C-cleanupCategory: PRs that clean code up or issues documenting cleanup.
and removed
C-enhancementCategory: An issue proposing an enhancement or a PR with one.
on May 2, 2020
RalfJung

RalfJung commented on May 2, 2020

@RalfJung
Member

Yeah something like that.

So we also use this issue to track cleaning up the error reporting story more generally? If we also include cleaning up this then I think we can close #67191, AFAIK that's all that's left.

changed the title [-]opportunistically evaluate constants while populating `mir::Body::required_consts`[/-] [+]opportunistically evaluate constants while populating `mir::Body::required_consts` + CTFE error cleanup[/+] on May 2, 2020
RalfJung

RalfJung commented on May 3, 2020

@RalfJung
Member

rust-lang/miri#1382 is also related, this is Miri lacking the code that codegen has to properly report post-monomorphization errors.

self-assigned this
on May 5, 2020

7 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)A-mir-optArea: MIR optimizationsA-mir-opt-inliningArea: MIR inliningC-cleanupCategory: PRs that clean code up or issues documenting cleanup.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @spastorino@RalfJung@oli-obk@jonas-schievink@tmiasko

        Issue actions

          opportunistically evaluate constants while populating `mir::Body::required_consts` · Issue #71802 · rust-lang/rust