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

Replace NonCopyable usage with NoPod #12016

Merged
merged 1 commit into from
Feb 4, 2014

Conversation

flaper87
Copy link
Contributor

@flaper87 flaper87 commented Feb 3, 2014

cc #10834

@flaper87
Copy link
Contributor Author

flaper87 commented Feb 3, 2014

@huonw r?


#[test]
fn test_replace() {
let mut x = Some(NonCopyable);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this testing replace, not NonCopyable?

Maybe change the test to something like Some(~"foo").

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erm, good catch!

@pnkfelix
Copy link
Member

pnkfelix commented Feb 3, 2014

Is it just me, or is "NoPod" less readable than "NonCopyable" ? :(

(I guess I did not mind it or notice so much when it was buried in the rest of PR #11768 but here it seems to bother me.)

I would have been fine if NonCopyable had stayed and we just changed its definition to be based upon having a NoPod field rather than a non_copyable attribute...

@flaper87
Copy link
Contributor Author

flaper87 commented Feb 3, 2014

@pnkfelix you mean just having a NonCopyable struct with a NoPod field and without the destructor?

@flaper87
Copy link
Contributor Author

flaper87 commented Feb 3, 2014

Also, FWIW, it doesn't bothers me much. TBH!

@pnkfelix
Copy link
Member

pnkfelix commented Feb 3, 2014

@flaper87 yes, a field and deleting the destructor is what I meant. (In my earlier comment I had forgotten the current implementation strategy for NonCopyable (a destructor plus a no-drop-flag attribute.)

Anyway I was just voicing my personal taste, I do not care enough about the readability issue here to try to block your change. After all my own taste might change in time.

@flaper87
Copy link
Contributor Author

flaper87 commented Feb 3, 2014

@alexcrichton r?

@bors bors closed this Feb 4, 2014
@bors bors merged commit c6b1bce into rust-lang:master Feb 4, 2014
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 26, 2024
stop linting [`blocks_in_conditions`] on `match` with weird attr macro case

should fixes: rust-lang#12016

---

changelog: [`blocks_in_conditions`] - fix FP on `match` with weird attr macro

This might not be the best solution, as the root cause (i think?) is the `span` of block was incorrectly given by the compiler?

I'm open to better solutions
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.

6 participants