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

ICE when a static variable is declared of type for which the Drop trait is implemented #9243

Closed
MarkJr94 opened this issue Sep 17, 2013 · 15 comments
Labels
I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority
Milestone

Comments

@MarkJr94
Copy link
Contributor

Minimal test case:

struct Test {
    mem: int,
}

pub static g_test: Test = Test {
    mem: 0,
};

impl Drop for Test {
    fn drop(&self) {
    }
}

fn main() {

}

Resulting Error after run with RUST_LOG=rustc=1 rustc test.rs:

task <unnamed> failed at 'assertion failed: `(left == right) && (right == left)` (left: `1u`, right: `2u`)', /build/buildd/rust-nightly-201309160805~3fbfab0~raring/src/librustc/middle/trans/adt.rs:545
error: internal compiler error: unexpected failure
note: the compiler hit an unexpected failure path. this is a bug
note: try running with RUST_LOG=rustc=1 to get further details and report the results to github.com/mozilla/rust/issues
task <unnamed> failed at 'explicit failure', /build/buildd/rust-nightly-201309160805~3fbfab0~raring/src/librustc/rustc.rs:377

Also, this:

struct Test;

pub static g_test: Test = Test;

impl Drop for Test {
    fn drop(&self) {
    }
}

fn main() {

}

compiles just fine, although I believe both cases should give a compilation error about having static variable implementing the Drop trait.

My rustc is acquired form the rust-nightly package in the Ubuntu repo.

Output of rustc --version:

rustc 0.8-pre
host: x86_64-unknown-linux-gnu
@alexcrichton
Copy link
Member

Nominating the decision of whether to allow this for the well-defined milestone

@luqmana
Copy link
Member

luqmana commented Sep 17, 2013

This also raises the question about whether to even allow putting types with destructors in a static? When would they run?

@catamorphism
Copy link
Contributor

accepted for well-defined

@alchemy
Copy link

alchemy commented Sep 25, 2013

Static items of a type implementing the Drop trait can have their distructors called at program termination, as in C++. Thus, both test cases above should compile without errors.

@brson
Copy link
Contributor

brson commented Sep 27, 2013

@alchemy it's not clear what the semantics of running destructors at program termination would be since at program termination there are no tasks in which to run the destructor.

I'd say to just forbid types that implement Drop in statics, but that doesn't seem sufficient. What about mutable statics that contain objects (or closures) that might implement Drop? What about mutable types that contain allocations? Are we happy just letting them leak at termination?

@brson brson mentioned this issue Sep 27, 2013
@alexcrichton
Copy link
Member

We decided in the 10/15/13 meeting to disallow static items which have destructors (things like ~T or those with the Drop trait implemented).

@alchemy
Copy link

alchemy commented Oct 17, 2013

Wise decision, i have been a little too hasty in my previous comment.
As for the questions raised by @brson: as i see it, the only remaining issue is about managed allocations and potential memory leaks at program termination (as there is no task local gc). I'd say to leave things as they are. The compiler already forbids the @ operator in constant expressions, so the only way for allocations to end up in a (mutable) static item is through unsafe code, which seems reasonable.
The alternative is to disallow static items containing managed pointers altogether.

@Aatch
Copy link
Contributor

Aatch commented Dec 13, 2013

With the new Pod kind incoming, would making the rule be that statics have to be Pod be acceptable? I think that it's technically a bit more limiting, but probably not enough to matter. It should also prevent any other future odd edge cases.

@nrc
Copy link
Member

nrc commented Feb 14, 2014

still ICEs, looking into this

@huonw
Copy link
Member

huonw commented Feb 14, 2014

@nick29581 this seems very likely to be covered by #11979.

@nrc
Copy link
Member

nrc commented Feb 14, 2014

Thanks huonw, I knew there was another issue similar to this because we discussed it at the meeting, but I couldn't find the number.

@flaper87 - will this get fixed by #11979?

@nikomatsakis
Copy link
Contributor

Closing as dup of #11979 -- under the rules we've discussed there, both of these programs would be in error (no static value may have a destructor).

@nikomatsakis
Copy link
Contributor

Er, I see that #11979 is a PR, not an issue. Reopening :)

@nikomatsakis nikomatsakis reopened this Feb 14, 2014
@nikomatsakis
Copy link
Contributor

I guess the issue is #10577

@flaper87
Copy link
Contributor

@nick29581 #11979 fixes this, I'll keep this issue open since it refers to Trait that implement Drop specifically.

Aaron1011 added a commit to Aaron1011/rust that referenced this issue Mar 7, 2021
Output of `git log --oneline  c68432f1e..970bc67c3`:

970bc67c3 (HEAD, origin/master, origin/auto-cargo, origin/HEAD) Auto merge of rust-lang#9243 - wickerwaka:configurable-env-doc, r=ehuss
4d7a29b75 Document the configurable-env usntable option
f7a7a3f91 Auto merge of rust-lang#9229 - alexcrichton:fix-borrow-mut, r=ehuss
3f2ece7a9 Fix a `BorrowMut` error when stdout is closed
7441e8c23 Auto merge of rust-lang#8825 - Aaron1011:feature/report-future-incompat, r=ehuss
139ed73f5 Add future-incompat tracking issue number.
9ea350368 Fix some minor formatting issues.
f03d47ce4 Address review comments
6177c6584 Implement future incompatibility report support
c69409658 Auto merge of rust-lang#9022 - nagisa:nagisa/manifest_path, r=alexcrichton
548300b20 Add the path to the manifest in json output
99e714c05 Auto merge of rust-lang#9230 - kornelski:nobinaries, r=alexcrichton
61a31bc5f Auto merge of rust-lang#9236 - kornelski:track-assert, r=Eh2406
3f7f0942c track_caller on custom assert functions
6977dee10 Explain `cargo install` is not for libraries
e4aebf0a0 Auto merge of rust-lang#9231 - joshtriplett:clear-to-eol-if-color, r=alexcrichton
b219f0eb7 Auto merge of rust-lang#9181 - jyn514:computer-says-no, r=ehuss
0b1816578 Remove unhelpful link to Cargo book
ea46f5ce3 Use ANSI clear-to-EOL if color is force-enabled
a6394bcc1 Remove unnecessary `config` argument to `Features::add`
3a86ecf2d Fix TODO about nightly features
09677c83c Be less unix-centric in error messages
ecfdced0d Fix test that assumed tests always were run on the stable channel
eba541994 Update comment in build_script_env
a5720117f Make `nightly_features_allowed` a field instead of a function
169b09ce7 Compute `enable_nightly_features` once instead of on each call
8fc86e155 Remove unused thread_locals
4b096beae Fix `masquerade_as_nightly_cargo` in work threads
e56417c8c Suggest RUSTC_BOOTSTRAP=crate instead of RUSTC_BOOTSTRAP=1
418129dae Downgrade error to a warning when `RUSTC_BOOTSTRAP` is set or this is the nightly channel
6c422a2c0 Restrict RUSTC_BOOTSTRAP in build.rs
m-ou-se added a commit to m-ou-se/rust that referenced this issue Mar 9, 2021
Update Cargo

Output of `git log --oneline  c68432f1e..970bc67c3`:

970bc67c3 (HEAD, origin/master, origin/auto-cargo, origin/HEAD) Auto merge of rust-lang#9243 - wickerwaka:configurable-env-doc, r=ehuss
4d7a29b75 Document the configurable-env usntable option
f7a7a3f91 Auto merge of rust-lang#9229 - alexcrichton:fix-borrow-mut, r=ehuss
3f2ece7a9 Fix a `BorrowMut` error when stdout is closed
7441e8c23 Auto merge of rust-lang#8825 - Aaron1011:feature/report-future-incompat, r=ehuss
139ed73f5 Add future-incompat tracking issue number.
9ea350368 Fix some minor formatting issues.
f03d47ce4 Address review comments
6177c6584 Implement future incompatibility report support
c69409658 Auto merge of rust-lang#9022 - nagisa:nagisa/manifest_path, r=alexcrichton
548300b20 Add the path to the manifest in json output
99e714c05 Auto merge of rust-lang#9230 - kornelski:nobinaries, r=alexcrichton
61a31bc5f Auto merge of rust-lang#9236 - kornelski:track-assert, r=Eh2406
3f7f0942c track_caller on custom assert functions
6977dee10 Explain `cargo install` is not for libraries
e4aebf0a0 Auto merge of rust-lang#9231 - joshtriplett:clear-to-eol-if-color, r=alexcrichton
b219f0eb7 Auto merge of rust-lang#9181 - jyn514:computer-says-no, r=ehuss
0b1816578 Remove unhelpful link to Cargo book
ea46f5ce3 Use ANSI clear-to-EOL if color is force-enabled
a6394bcc1 Remove unnecessary `config` argument to `Features::add`
3a86ecf2d Fix TODO about nightly features
09677c83c Be less unix-centric in error messages
ecfdced0d Fix test that assumed tests always were run on the stable channel
eba541994 Update comment in build_script_env
a5720117f Make `nightly_features_allowed` a field instead of a function
169b09ce7 Compute `enable_nightly_features` once instead of on each call
8fc86e155 Remove unused thread_locals
4b096beae Fix `masquerade_as_nightly_cargo` in work threads
e56417c8c Suggest RUSTC_BOOTSTRAP=crate instead of RUSTC_BOOTSTRAP=1
418129dae Downgrade error to a warning when `RUSTC_BOOTSTRAP` is set or this is the nightly channel
6c422a2c0 Restrict RUSTC_BOOTSTRAP in build.rs
flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 28, 2022
Don't lint `std_instead_of_core` on `std::env`

fixes rust-lang#9239

This also reorders the execution of the lint to do as little as possible in the case where the path doesn't start with `std` or `alloc`.

changelog: [`std_instead_of_core`](https://rust-lang.github.io/rust-clippy/master/#std_instead_of_core): Don't lint on `use std::env`
changelog: [`std_instead_of_alloc`](https://rust-lang.github.io/rust-clippy/master/#std_instead_of_alloc): Don't lint `use std::vec` twice
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority
Projects
None yet
Development

No branches or pull requests