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

Fix issue #9243 #9570

Closed
wants to merge 1 commit into from
Closed

Fix issue #9243 #9570

wants to merge 1 commit into from

Conversation

alchemy
Copy link

@alchemy alchemy commented Sep 27, 2013

No description provided.

@alexcrichton
Copy link
Member

Thanks for the patch! The tests and the code all look good to me.

That being said, I think that this is a deeper issue which needs discussion before merging. I was under the impression that it wasn't clear what it meant to create a static variable with a destructor. Does this mean that the destructor is never run? Does this produce unsound behavior? Also what if it's a static mut? Are there more problems with moving in/out of the static mut or is it an even worse problem that the destructor is never run in this situation? (memory leaks are very possible).

I think that this needs to be discussed further to hammer down the semantics of statics with destructors before we can decide on a course of action.

@thestinger
Copy link
Contributor

cc @pcwalton, it was my impression that we didn't want this sort of thing

@brson
Copy link
Contributor

brson commented Sep 27, 2013

Agree that #9243 has a lot of subtlety that needs to be ironed out before moving forward.

@alchemy
Copy link
Author

alchemy commented Sep 28, 2013

Thanks for the review Alex.

I took the issue more or less at random, and just went on to fix it, but i see your point.

@alexcrichton
Copy link
Member

Hm I'm not sure why, but this is causing bors some trouble. I'm closing temporarily to see if this un-sticks bors.

@alexcrichton
Copy link
Member

Nope, didn't help

@alexcrichton alexcrichton reopened this Oct 7, 2013
@alexcrichton
Copy link
Member

Thanks again for this, but we decided in the 10/15/13 meeting that we shouldn't be allowing statics with destructors.

@alchemy alchemy deleted the issue-9243 branch December 10, 2013 14:53
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 21, 2022
…ction, r=llogiq

feat: lint unchecked subtraction of a 'Duration' from an 'Instant'

Hello all, I tried to tackle the open issue rust-lang#9371 and this is what I came up with.

I have a difficulty currently - some tests are failing:

```
failures:
    [ui] ui/manual_instant_elapsed.rs
```

The `manual_instant_elapsed` is failing because of `Instant::now() - duration` test, this now gets also picked by `unchecked_duration_subtraction` lint.
What is the correct way to proceed in this case? Simply update the `.stderr` file for `manual_instant_elapsed` lint?

changelog: [`unchecked_duration_subtraction`]: Add lint for unchecked subtraction of a `Duration` from an `Instant`.

fixes rust-lang#9371
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants