Document unsafe blocks in core::fmt #69011

foeb · 2020-02-10T00:50:58Z

r? @RalfJung
CC: @rust-lang/wg-unsafe-code-guidelines
#66219

Sorry for the hiatus, but here's a few more files with the unsafe blocks documented! I think working on it smaller chunks like this will be easier for everyone.

Mark-Simulacrum · 2020-02-10T01:31:18Z

I'm relatively familiar with this code and can likely do a review here, so let's r? @Mark-Simulacrum to take some work off @RalfJung.

Mark-Simulacrum

Generally fine to resolve most of the comments with just adding FIXMEs when it comes to changing the code, I think.

I also pinged folks on the comments I was not sure about.

src/libcore/fmt/mod.rs

src/libcore/fmt/num.rs

eddyb · 2020-02-11T18:46:47Z

cc @rust-lang/wg-unsafe-code-guidelines (I believe the mention doesn't work for non-members)

Mark-Simulacrum

r=me with comment fixed and commits squashed into one

Mark-Simulacrum · 2020-02-18T02:59:46Z

src/libcore/fmt/float.rs

@@ -16,6 +14,7 @@ fn float_to_decimal_common_exact<T>(
 where
    T: flt2dec::DecodableFloat,
 {
+    // SAFETY: Possible undefined behavior, see FIXME


Could we make these refer to some marker, e.g., FIXME(MaybeUninit) or an issue number or so, and then use that in the actual FIXME as well?

The FIXMEs already point to #53491, does that work or would it be better if changed these lines to // SAFETY: Possible undefined behavior, see FIXME(#53491) also?

Yes, exactly, currently it's not clear which FIXME this is telling you to see.

I believe this is the only thing left to resolve here.

rust-highfive · 2020-02-18T03:03:04Z

The job x86_64-gnu-llvm-7 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.

2020-02-18T02:46:38.6554615Z ========================== Starting Command Output ===========================
2020-02-18T02:46:38.6557639Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/e1f55b3f-f673-42c7-a7b8-407a4e04c40b.sh
2020-02-18T02:46:38.6557814Z 
2020-02-18T02:46:38.6560720Z ##[section]Finishing: Disable git automatic line ending conversion
2020-02-18T02:46:38.6567463Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69011/merge to s
2020-02-18T02:46:38.6569016Z Task         : Get sources
2020-02-18T02:46:38.6569045Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-18T02:46:38.6569073Z Version      : 1.0.0
2020-02-18T02:46:38.6569118Z Author       : Microsoft
---
2020-02-18T02:46:39.5557548Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-02-18T02:46:39.5639253Z ##[command]git config gc.auto 0
2020-02-18T02:46:39.5716505Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-02-18T02:46:39.5774122Z ##[command]git config --get-all http.proxy
2020-02-18T02:46:39.5947524Z ##[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/69011/merge:refs/remotes/pull/69011/merge
---
2020-02-18T02:52:51.7687788Z    Compiling serde_json v1.0.40
2020-02-18T02:52:53.4113845Z    Compiling tidy v0.1.0 (/checkout/src/tools/tidy)
2020-02-18T02:53:04.0637519Z     Finished release [optimized] target(s) in 1m 36s
2020-02-18T02:53:04.0725525Z tidy check
2020-02-18T02:53:04.4594737Z tidy error: /checkout/src/libcore/fmt/num.rs:330: undocumented unsafe
2020-02-18T02:53:04.4594818Z tidy error: /checkout/src/libcore/fmt/num.rs:341: undocumented unsafe
2020-02-18T02:53:04.4595296Z tidy error: /checkout/src/libcore/fmt/num.rs:350: undocumented unsafe
2020-02-18T02:53:04.4595436Z tidy error: /checkout/src/libcore/fmt/num.rs:355: undocumented unsafe
2020-02-18T02:53:04.4595480Z tidy error: /checkout/src/libcore/fmt/num.rs:367: undocumented unsafe
2020-02-18T02:53:06.5656466Z some tidy checks failed
2020-02-18T02:53:06.5656558Z Found 487 error codes
2020-02-18T02:53:06.5656603Z Found 0 error codes with no tests
2020-02-18T02:53:06.5656888Z Done!
2020-02-18T02:53:06.5656888Z Done!
2020-02-18T02:53:06.5656915Z 
2020-02-18T02:53:06.5657116Z 
2020-02-18T02:53:06.5658455Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor"
2020-02-18T02:53:06.5658582Z 
2020-02-18T02:53:06.5658607Z 
2020-02-18T02:53:06.5667489Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2020-02-18T02:53:06.5667602Z Build completed unsuccessfully in 0:01:47
2020-02-18T02:53:06.5667602Z Build completed unsuccessfully in 0:01:47
2020-02-18T02:53:06.5715611Z == clock drift check ==
2020-02-18T02:53:06.5724244Z   local time: Tue Feb 18 02:53:06 UTC 2020
2020-02-18T02:53:06.8571491Z   network time: Tue, 18 Feb 2020 02:53:06 GMT
2020-02-18T02:53:06.8576351Z == end clock drift check ==
2020-02-18T02:53:07.6202638Z 
2020-02-18T02:53:07.6306625Z ##[error]Bash exited with code '1'.
2020-02-18T02:53:07.6319775Z ##[section]Finishing: Run build
2020-02-18T02:53:07.6336259Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69011/merge to s
2020-02-18T02:53:07.6337976Z Task         : Get sources
2020-02-18T02:53:07.6338036Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-18T02:53:07.6338079Z Version      : 1.0.0
2020-02-18T02:53:07.6338117Z Author       : Microsoft
2020-02-18T02:53:07.6338117Z Author       : Microsoft
2020-02-18T02:53:07.6338178Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-02-18T02:53:07.6338241Z ==============================================================================
2020-02-18T02:53:08.0489727Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-02-18T02:53:08.0531349Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/69011/merge to s
2020-02-18T02:53:08.0641724Z Cleaning up task key
2020-02-18T02:53:08.0642975Z Start cleaning up orphan processes.
2020-02-18T02:53:08.0747635Z Terminate orphan process: pid (4892) (python)
2020-02-18T02:53:08.0944435Z ##[section]Finishing: Finalize Job

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)

foeb · 2020-02-18T03:17:09Z

It looks like a new macro with some unsafe blocks was added to master. I'm fixing that now.

bors · 2020-02-26T19:27:05Z

☔ The latest upstream changes (presumably #69484) made this pull request unmergeable. Please resolve the merge conflicts.

Mark-Simulacrum · 2020-02-28T01:44:05Z

Looks like this needs a rebase -- which will remove a few of the FIXMEs -- and otherwise I think is good to go.

Mark-Simulacrum

Please also squash into one commit.

Mark-Simulacrum · 2020-03-02T18:48:48Z

src/libcore/fmt/float.rs

@@ -16,6 +14,7 @@ fn float_to_decimal_common_exact<T>(
 where
    T: flt2dec::DecodableFloat,
 {
+    // SAFETY: Possible undefined behavior, see FIXME


I believe this is the only thing left to resolve here.

foeb · 2020-03-11T04:15:39Z

@Mark-Simulacrum This is ready for review again. Thanks!

Mark-Simulacrum · 2020-03-11T13:04:15Z

@bors r+

Thanks!

bors · 2020-03-11T13:04:16Z

📌 Commit 3d146a3 has been approved by Mark-Simulacrum

@RalfJung

…rk-Simulacrum Document unsafe blocks in core::fmt r? @RalfJung CC: @rust-lang/wg-unsafe-code-guidelines rust-lang#66219 Sorry for the hiatus, but here's a few more files with the unsafe blocks documented! I think working on it smaller chunks like this will be easier for everyone.

@RalfJung

…rk-Simulacrum Document unsafe blocks in core::fmt r? @RalfJung CC: @rust-lang/wg-unsafe-code-guidelines rust-lang#66219 Sorry for the hiatus, but here's a few more files with the unsafe blocks documented! I think working on it smaller chunks like this will be easier for everyone.

@ghost

Rollup of 10 pull requests Successful merges: - #68899 (Add Display and Error impls for proc_macro::LexError) - #69011 (Document unsafe blocks in core::fmt) - #69674 (Rename DefKind::Method and TraitItemKind::Method ) - #69705 (Toolstate: remove redundant beta-week check.) - #69722 (Tweak output for invalid negative impl AST errors) - #69747 (Rename rustc guide) - #69792 (Implement Error for TryReserveError) - #69830 (miri: ICE on invalid terminators) - #69921 (rustdoc: remove unused import) - #69945 (update outdated comment) Failed merges: r? @ghost

rust-highfive assigned RalfJung Feb 10, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 10, 2020

rust-highfive assigned Mark-Simulacrum and unassigned RalfJung Feb 10, 2020

Mark-Simulacrum reviewed Feb 11, 2020

View reviewed changes

Mark-Simulacrum approved these changes Feb 18, 2020

View reviewed changes

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 28, 2020

Mark-Simulacrum reviewed Mar 2, 2020

View reviewed changes

Document unsafe blocks in core::fmt

3d146a3

foeb force-pushed the document-unsafe-core-fmt branch from c3106f9 to 3d146a3 Compare March 6, 2020 03:55

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 11, 2020

Centril mentioned this pull request Mar 11, 2020

Rollup of 8 pull requests #69927

Closed

Centril mentioned this pull request Mar 11, 2020

Rollup of 9 pull requests #69931

Closed

Centril mentioned this pull request Mar 12, 2020

Rollup of 10 pull requests #69950

Merged

bors merged commit 156a05a into rust-lang:master Mar 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document unsafe blocks in core::fmt #69011

Document unsafe blocks in core::fmt #69011

foeb commented Feb 10, 2020

Mark-Simulacrum commented Feb 10, 2020

Mark-Simulacrum left a comment

eddyb commented Feb 11, 2020

Mark-Simulacrum left a comment

Mark-Simulacrum Feb 18, 2020

foeb Feb 18, 2020

Mark-Simulacrum Feb 18, 2020

Mark-Simulacrum Mar 2, 2020

rust-highfive commented Feb 18, 2020

foeb commented Feb 18, 2020

bors commented Feb 26, 2020

Mark-Simulacrum commented Feb 28, 2020

Mark-Simulacrum left a comment

Mark-Simulacrum Mar 2, 2020

foeb commented Mar 11, 2020

Mark-Simulacrum commented Mar 11, 2020

bors commented Mar 11, 2020

Document unsafe blocks in core::fmt #69011

Document unsafe blocks in core::fmt #69011

Conversation

foeb commented Feb 10, 2020

Mark-Simulacrum commented Feb 10, 2020

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

eddyb commented Feb 11, 2020

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

Mark-Simulacrum Feb 18, 2020

Choose a reason for hiding this comment

foeb Feb 18, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Feb 18, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Mar 2, 2020

Choose a reason for hiding this comment

rust-highfive commented Feb 18, 2020

foeb commented Feb 18, 2020

bors commented Feb 26, 2020

Mark-Simulacrum commented Feb 28, 2020

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

Mark-Simulacrum Mar 2, 2020

Choose a reason for hiding this comment

foeb commented Mar 11, 2020

Mark-Simulacrum commented Mar 11, 2020

bors commented Mar 11, 2020