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

Associated const references using UFCS can bypass check_static_recursion #24949

Open
quantheory opened this issue Apr 29, 2015 · 6 comments · Fixed by #71657
Open

Associated const references using UFCS can bypass check_static_recursion #24949

quantheory opened this issue Apr 29, 2015 · 6 comments · Fixed by #71657
Labels
A-associated-items Area: Associated items (types, constants & functions) A-type-system Area: Type system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@quantheory
Copy link
Contributor

quantheory commented Apr 29, 2015

Currently, check_static_recursion is run after resolve, but before typeck. This made sense before, because after resolve it was possible to determine which constants referenced one another, and running before typeck ensured that we don't fall into an infinite loop during type checking. However, when referencing an associated constant with some UFCS forms, we can't resolve the constant until typeck, meaning that check_static_recursion can't always tell when a recursive definition is present. On top of that, this check is not currently working right even for inherent impls, where it should be possible to discover a problem ahead of typeck running.

A set of test cases:

// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(associated_consts)]

trait Foo {
    const BAR: u32;
}

// Check for recursion involving references to trait-associated const.

const TRAIT_REF_BAR: u32 = <GlobalTraitRef>::BAR; //~ ERROR E0265

struct GlobalTraitRef;

impl Foo for GlobalTraitRef {
    const BAR: u32 = TRAIT_REF_BAR; //~ ERROR E0265
}

// Check for recursion involving references to impl-associated const.

const IMPL_REF_BAR: u32 = GlobalImplRef::BAR; //~ ERROR E0265

struct GlobalImplRef;

impl GlobalImplRef {
    const BAR: u32 = IMPL_REF_BAR; //~ ERROR E0265
}

// Check for recursion involving references to trait-associated const default.

trait FooDefault {
    const BAR: u32 = DEFAULT_REF_BAR; //~ ERROR E0265
}

const DEFAULT_REF_BAR: u32 = <GlobalDefaultRef>::BAR; //~ ERROR E0265

struct GlobalDefaultRef;

impl FooDefault for GlobalDefaultRef {}

fn main() {}

The above cases should all fail in the recursion check, but they all compile successfully! If any of the constants are actually used in a context that requires the compiler to evaluate them, there will be an ICE (specifically, the compiler enters an infinite loop that ends up blowing the stack).

Update
playground link: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=76e9edaf2827806e18a557f37bc930ea

This issue has been assigned to @Daniel-Worrall via this comment.

@steveklabnik steveklabnik added A-type-system Area: Type system A-associated-items Area: Associated items (types, constants & functions) labels Apr 30, 2015
@Gankra
Copy link
Contributor

Gankra commented Jun 2, 2015

A simpler example of this:

trait Foo {
    const FOO: Self;
}

impl Foo for u8 {
    const FOO: u8 = u8::FOO;
}

compiles.

@steveklabnik
Copy link
Member

Triage: @gankro 's example still compiles.

@Mark-Simulacrum
Copy link
Member

E-needstest.

@Mark-Simulacrum Mark-Simulacrum added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label May 18, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 22, 2017
@varkor
Copy link
Member

varkor commented Feb 25, 2019

trait Foo {
    const FOO: Self;
}

impl Foo for u8 {
    const FOO: u8 = u8::FOO;
}

fails to compile, but adding:

fn main() {}

makes it compile without an error.

@Daniel-Worrall
Copy link
Contributor

@rustbot claim

@rustbot rustbot self-assigned this Apr 27, 2020
Daniel-Worrall added a commit to Daniel-Worrall/rust that referenced this issue Apr 29, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 29, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#71217 (Suggest `;` or assignment to drop borrows in tail exprs)
 - rust-lang#71286 (Add regression test for rust-lang#69654)
 - rust-lang#71296 (Change wording on read_vectored docs)
 - rust-lang#71654 (Update link to unstable book for llvm_asm macro)
 - rust-lang#71657 (Add rust-lang#24949 assoc constant static recursion test)

Failed merges:

r? @ghost
@bors bors closed this as completed in 878e928 Apr 29, 2020
@varkor
Copy link
Member

varkor commented Apr 29, 2020

This isn't fixed. The example in this comment still compiles.

@varkor varkor reopened this Apr 29, 2020
@Dylan-DPC-zz Dylan-DPC-zz removed the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Apr 29, 2020
@Noratrieb Noratrieb added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-type-system Area: Type system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants