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

Fixed PhantomData markers in Arc and Rc #66117

Merged
merged 3 commits into from
Nov 7, 2019

Conversation

olegnn
Copy link
Contributor

@olegnn olegnn commented Nov 5, 2019

Include owned internal structs in PhantomData markers in Arc (PhantomData<T> => PhantomData<ArcInner<T>>) and Rc (PhantomData<T> => PhantomData<RcBox<T>>).

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 5, 2019
@Mark-Simulacrum
Copy link
Member

This is probably a little above my head -- cc @RalfJung

and r? @alexcrichton

@RalfJung
Copy link
Member

RalfJung commented Nov 5, 2019

It includes internal struct Node which is not exposed (in Arc and Rc structs ArcInner and RcBox aren't included in marker)

I'd argue that Arc and Rc should be changed then. This is not about "what is exposed" but about "what is owned".

It includes Box which is actually consumed when value is stored in list and used only to allocate memory on push and deallocate on pop

Aren't these NonNull pointers Boxes? In that case, it seems reasonable to say that this node owns a Box and that PhantomData should reflect that.

@olegnn
Copy link
Contributor Author

olegnn commented Nov 5, 2019

Just to add candidate to be fixed:

In case of LinkedList, unlike in Arc or Rc, we are sure that NonNull pointer may be only received from Box::into_raw_non_null call, so use of PhantomData<Box<...>> even if Box doesn't actually exist in our type system but takes place in memory (PhantomData definition) makes sense.

However NonNull received from Box::into_raw_non_null due to std::mem::forget used in Box::into_unique doesn't require any PhantomData to stop dropck from deleting value (Playground), it's here just to indicate that LinkedList owns T and can call destructor on its value when LinkedList will be dropped.

@RalfJung
Copy link
Member

RalfJung commented Nov 5, 2019

What do you mean by "stop dropck from deleting value"? dropck needs to detect possible cycles and, in particular, it needs to know which other types will be dropped when this one is dropped. Dropping a Node in the linked list deallocates a Box<T>, potentially, right? If yes, there should be a PhantomData<Box<T>>.

@alexcrichton
Copy link
Member

I think @RalfJung knows more about this than I do!

r? @RalfJung

@olegnn olegnn changed the title LinkedList: PhantomData<Box<Node<T>>> => PhantomData<T> Fixed PhantomData markers in Arc and Rc Nov 5, 2019
@RalfJung
Copy link
Member

RalfJung commented Nov 5, 2019

Thanks, this looks right to me! Once CI is green, can you do bors r=RalfJung?

@bors delegate+ rollup

@bors
Copy link
Contributor

bors commented Nov 5, 2019

✌️ @olegnn can now approve this pull request

1 similar comment
@bors
Copy link
Contributor

bors commented Nov 5, 2019

✌️ @olegnn can now approve this pull request

@olegnn
Copy link
Contributor Author

olegnn commented Nov 5, 2019

Sure! Thank you for explanation.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-11-05T20:41:45.4183921Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-11-05T20:41:45.4403049Z ##[command]git config gc.auto 0
2019-11-05T20:41:45.4468434Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-11-05T20:41:45.4517665Z ##[command]git config --get-all http.proxy
2019-11-05T20:41:45.4673815Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/66117/merge:refs/remotes/pull/66117/merge
---
2019-11-05T21:33:41.5653106Z .................................................................................................... 1600/9276
2019-11-05T21:33:46.6035846Z .................................................................................................... 1700/9276
2019-11-05T21:33:57.6130111Z ...............................................................i...............i.................... 1800/9276
2019-11-05T21:34:04.0103876Z .................................................................................................... 1900/9276
2019-11-05T21:34:18.1733820Z .....................................................iiiii.......................................... 2000/9276
2019-11-05T21:34:27.6664548Z .................................................................................................... 2200/9276
2019-11-05T21:34:29.8727178Z .................................................................................................... 2300/9276
2019-11-05T21:34:32.8049364Z .................................................................................................... 2400/9276
2019-11-05T21:34:52.6249291Z .................................................................................................... 2500/9276
2019-11-05T21:34:52.6249291Z .................................................................................................... 2500/9276
2019-11-05T21:34:55.0311925Z .................................................................................................... 2600/9276
2019-11-05T21:35:01.6296253Z .................................................................................................... 2700/9276
2019-11-05T21:35:09.3647830Z .....................i.............................................................................. 2800/9276
2019-11-05T21:35:17.0812267Z .................................................................................................... 2900/9276
2019-11-05T21:35:21.0232918Z ....................i............................................................................... 3000/9276
2019-11-05T21:35:28.6215607Z .................................................................................................... 3100/9276
2019-11-05T21:35:33.3933804Z .................................................................................................... 3200/9276
2019-11-05T21:35:41.1109263Z ..ii................................................................................................ 3300/9276
2019-11-05T21:35:55.6603653Z ...............................................................................................i.... 3500/9276
2019-11-05T21:36:02.0194805Z ..........................................i......................................................... 3600/9276
2019-11-05T21:36:07.8630206Z .................................................................................................... 3700/9276
2019-11-05T21:36:13.5325327Z .................................................................................................... 3800/9276
---
2019-11-05T21:37:22.0762589Z .....................................................i...............i.............................. 4800/9276
2019-11-05T21:37:30.1905471Z .................................................................................................... 4900/9276
2019-11-05T21:37:38.4230816Z .................................................................................................... 5000/9276
2019-11-05T21:37:43.6896461Z .................................................................................................... 5100/9276
2019-11-05T21:37:53.0100274Z ......................................................ii.ii...........i............................. 5200/9276
2019-11-05T21:38:02.0728304Z .................................................................................................... 5400/9276
2019-11-05T21:38:11.2352444Z .................................................................................................... 5500/9276
2019-11-05T21:38:18.2316664Z ...........................i........................................................................ 5600/9276
2019-11-05T21:38:24.2196819Z .................................................................................................... 5700/9276
2019-11-05T21:38:24.2196819Z .................................................................................................... 5700/9276
2019-11-05T21:38:35.1581624Z .................................................................................................... 5800/9276
2019-11-05T21:38:45.6946302Z ............ii...i..ii...........i.................................................................. 5900/9276
2019-11-05T21:39:03.7236964Z .................................................................................................... 6100/9276
2019-11-05T21:39:11.7784047Z .................................................................................................... 6200/9276
2019-11-05T21:39:11.7784047Z .................................................................................................... 6200/9276
2019-11-05T21:39:24.0027112Z ...............................i..ii................................................................ 6300/9276
2019-11-05T21:39:42.1160818Z ..................................................................................................i. 6500/9276
2019-11-05T21:39:44.0226448Z .................................................................................................... 6600/9276
2019-11-05T21:39:45.9657698Z ............................................................................i....................... 6700/9276
2019-11-05T21:39:48.2716697Z .................................................................................................... 6800/9276
---
2019-11-05T21:44:27.8070533Z  finished in 5.574
2019-11-05T21:44:27.8234122Z Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-11-05T21:44:28.0296910Z 
2019-11-05T21:44:28.0298839Z running 158 tests
2019-11-05T21:44:30.9352741Z iiiiii....iii......iii..iiii...i.............................i..i..................i....i........... 100/158
2019-11-05T21:44:32.9062403Z ii.i.i..iiii..............i.........iii.i.........ii......
2019-11-05T21:44:32.9062893Z 
2019-11-05T21:44:32.9065351Z  finished in 5.082
2019-11-05T21:44:32.9232988Z Check compiletest suite=codegen-units mode=codegen-units (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-11-05T21:44:33.0747125Z 
---
2019-11-05T21:44:35.0311741Z  finished in 2.107
2019-11-05T21:44:35.0491593Z Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-11-05T21:44:35.1946434Z 
2019-11-05T21:44:35.1946645Z running 9 tests
2019-11-05T21:44:35.1947795Z iiiiiiiii
2019-11-05T21:44:35.1948380Z 
2019-11-05T21:44:35.1949341Z  finished in 0.145
2019-11-05T21:44:35.2114494Z Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-11-05T21:44:35.3719614Z 
---
2019-11-05T21:44:52.7678391Z  finished in 17.556
2019-11-05T21:44:52.7881910Z Check compiletest suite=debuginfo mode=debuginfo-gdb+lldb (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-11-05T21:44:52.9480408Z 
2019-11-05T21:44:52.9481001Z running 123 tests
2019-11-05T21:45:13.8382027Z .iiiii...i.....i..i...i..i.i.i..i.ii..i.i.....i..i....ii..........iiii..........i...ii...i.......ii. 100/123
2019-11-05T21:45:18.0413899Z i.i.i......iii.i.....ii
2019-11-05T21:45:18.0414508Z 
2019-11-05T21:45:18.0414551Z  finished in 25.018
2019-11-05T21:45:18.0414777Z Uplifting stage1 rustc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-11-05T21:45:18.0415395Z Copying stage2 rustc from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
---
2019-11-05T21:56:00.5395656Z 
2019-11-05T21:56:00.5409408Z    Doc-tests core
2019-11-05T21:56:05.4716973Z 
2019-11-05T21:56:05.4717920Z running 2410 tests
2019-11-05T21:56:15.2527969Z ......iiiii.......................F................................................................. 100/2410
2019-11-05T21:56:24.8037591Z ................................................................................ii.................. 200/2410
2019-11-05T21:56:46.9370028Z ..i................................................................................................. 400/2410
2019-11-05T21:56:46.9370028Z ..i................................................................................................. 400/2410
2019-11-05T21:56:55.9572098Z ..................................................i..i.................iiii......................... 500/2410
2019-11-05T21:57:13.4384856Z .................................................................................................... 700/2410
2019-11-05T21:57:22.3816433Z .................................................................................................... 800/2410
2019-11-05T21:57:31.6634525Z .................................................................................................... 900/2410
2019-11-05T21:57:40.9625351Z .................................................................................................... 1000/2410
---
2019-11-05T21:59:53.2409314Z ...................i................................................................................ 2400/2410
2019-11-05T21:59:54.2260632Z ..........
2019-11-05T21:59:54.2261230Z failures:
2019-11-05T21:59:54.2261485Z 
2019-11-05T21:59:54.2263571Z ---- cell.rs - cell (line 135) stdout ----
2019-11-05T21:59:54.2263633Z error[E0063]: missing field `phantom` in initializer of `main::Rc<_>`
2019-11-05T21:59:54.2263877Z   --> cell.rs:156:9
2019-11-05T21:59:54.2263915Z    |
2019-11-05T21:59:54.2263969Z 24 |         Rc { ptr: self.ptr }
2019-11-05T21:59:54.2264005Z    |         ^^ missing `phantom`
2019-11-05T21:59:54.2264063Z error: aborting due to previous error
2019-11-05T21:59:54.2264102Z 
2019-11-05T21:59:54.2264798Z For more information about this error, try `rustc --explain E0063`.
2019-11-05T21:59:54.2265001Z Couldn't compile the test.
---
2019-11-05T21:59:54.2266834Z 
2019-11-05T21:59:54.2298735Z error: test failed, to rerun pass '--doc'
2019-11-05T21:59:54.2314965Z 
2019-11-05T21:59:54.2315143Z 
2019-11-05T21:59:54.2316158Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "-Zconfig-profile" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/src/libtest/Cargo.toml" "-p" "core" "--" "--quiet"
2019-11-05T21:59:54.2317036Z 
2019-11-05T21:59:54.2317256Z 
2019-11-05T21:59:54.2325504Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
2019-11-05T21:59:54.2325602Z Build completed unsuccessfully in 1:12:03
2019-11-05T21:59:54.2325602Z Build completed unsuccessfully in 1:12:03
2019-11-05T21:59:54.2372538Z == clock drift check ==
2019-11-05T21:59:54.2389284Z   local time: Tue Nov  5 21:59:54 UTC 2019
2019-11-05T21:59:54.3103817Z   network time: Tue, 05 Nov 2019 21:59:54 GMT
2019-11-05T21:59:54.3106649Z == end clock drift check ==
2019-11-05T21:59:54.9042730Z 
2019-11-05T21:59:54.9153811Z ##[error]Bash exited with code '1'.
2019-11-05T21:59:54.9198822Z ##[section]Starting: Checkout
2019-11-05T21:59:54.9200247Z ==============================================================================
2019-11-05T21:59:54.9200289Z Task         : Get sources
2019-11-05T21:59:54.9200324Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@olegnn
Copy link
Contributor Author

olegnn commented Nov 6, 2019

@bors r=RalfJung

@bors
Copy link
Contributor

bors commented Nov 6, 2019

📌 Commit 6a59319 has been approved by RalfJung

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 6, 2019
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Nov 6, 2019
…RalfJung

Fixed PhantomData markers in Arc and Rc

Include owned internal structs in `PhantomData` markers in `Arc` (`PhantomData<T>` => `PhantomData<ArcInner<T>>`) and `Rc` (`PhantomData<T>` => `PhantomData<RcBox<T>>`).
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 6, 2019
…RalfJung

Fixed PhantomData markers in Arc and Rc

Include owned internal structs in `PhantomData` markers in `Arc` (`PhantomData<T>` => `PhantomData<ArcInner<T>>`) and `Rc` (`PhantomData<T>` => `PhantomData<RcBox<T>>`).
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 7, 2019
…RalfJung

Fixed PhantomData markers in Arc and Rc

Include owned internal structs in `PhantomData` markers in `Arc` (`PhantomData<T>` => `PhantomData<ArcInner<T>>`) and `Rc` (`PhantomData<T>` => `PhantomData<RcBox<T>>`).
bors added a commit that referenced this pull request Nov 7, 2019
Rollup of 12 pull requests

Successful merges:

 - #65794 (gate rustc_on_unimplemented under rustc_attrs)
 - #65945 (Optimize long-linker-command-line test)
 - #66044 (Improve uninit/zeroed lint)
 - #66076 (HIR docs: mention how to resolve method paths)
 - #66084 (Do not require extra LLVM backends for `x.py test` to pass)
 - #66111 (improve from_raw_parts docs)
 - #66114 (Improve std::thread::Result documentation)
 - #66117 (Fixed PhantomData markers in Arc and Rc)
 - #66146 (Remove unused parameters in `__thread_local_inner`)
 - #66147 (Miri: Refactor to_scalar_ptr out of existence)
 - #66162 (Fix broken link in README)
 - #66171 (Update link on CONTRIBUTING.md)

Failed merges:

r? @ghost
@bors bors merged commit 6a59319 into rust-lang:master Nov 7, 2019
cuviper added a commit to cuviper/darc that referenced this pull request Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants