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 C-variadic function printing #58865

Merged
merged 2 commits into from Mar 3, 2019
Merged

Conversation

dlrobertson
Copy link
Contributor

@dlrobertson dlrobertson commented Mar 2, 2019

There is no longer a need to append the string ", ..." to a functions
args as ... is parsed as an argument and will appear in the functions
arguments.

r? @alexreg
cc @alexcrichton
Fixes: #58853

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 2, 2019
Copy link
Contributor

@alexreg alexreg left a comment

Choose a reason for hiding this comment

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

Looks sound. (What I had in mind). Waiting for a regression test.

@alexreg alexreg 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 Mar 2, 2019
@dlrobertson
Copy link
Contributor Author

@alexreg added regression test

There is no longer a need to append the string `", ..."` to a functions
args as `...` is parsed as an argument and will appear in the functions
arguments.
@alexreg
Copy link
Contributor

alexreg commented Mar 2, 2019

@bors r+ p=1

@bors
Copy link
Contributor

bors commented Mar 2, 2019

📌 Commit 72f0ca5 has been approved by alexreg

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 2, 2019
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Mar 2, 2019
Fix C-variadic function printing

There is no longer a need to append the string `", ..."` to a functions
args as `...` is parsed as an argument and will appear in the functions
arguments.

r? @alexreg
cc @alexcrichton
Fixes: rust-lang#58853
@bors
Copy link
Contributor

bors commented Mar 3, 2019

⌛ Testing commit 72f0ca5 with merge 988f967fd8f122244f5463bb6e450eb56539232c...

@bors
Copy link
Contributor

bors commented Mar 3, 2019

💔 Test failed - checks-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 3, 2019
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-aux of your PR failed on Travis (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.
[01:27:16] test [pretty] pretty/empty-impl.rs ... ok
[01:27:16] test [pretty] pretty/example1.rs ... ok
[01:27:16] test [pretty] pretty/example2.rs ... ok
[01:27:16] test [pretty] pretty/empty-lines.rs ... ok
[01:27:16] ERROR 2019-03-03T02:59:23Z: compiletest::runtest: fatal error, panic: "pretty-printed source does not match expected source\nexpected:\n------------------------------------------\n// Check that `fn foo(x: i32, ...)` does not print as `fn foo(x: i32, ..., ...)`.\n// See issue #58853.\n\n// pp-exact\n#![feature(c_variadic)]\n\nextern \"C\" {\n    pub fn foo(x: i32, ...);\n}\n\npub unsafe extern \"C\" fn bar(_: i32, mut ap: ...) -> usize {\n    ap.arg::<usize>()\n}\n\nfn main() {}\n\n------------------------------------------\nactual:\n------------------------------------------\n// Check that `fn foo(x: i32, ...)` does not print as `fn foo(x: i32, ..., ...)`.\n// See issue #58853.\n\n// pp-exact\n#![feature(c_variadic)]\n\nextern \"C\" {\n    pub fn foo(x: i32, ...);\n}\n\npub unsafe extern \"C\" fn bar(_: i32, mut ap: ...) -> usize {\n    ap.arg::<usize>()\n}\n\nfn main() { }\n\n------------------------------------------\n\n"
[01:27:16] test [pretty] pretty/fn-return.rs ... ok
[01:27:16] test [pretty] pretty/fn-types.rs ... ok
[01:27:16] test [pretty] pretty/import-renames.rs ... ok
[01:27:16] test [pretty] pretty/for-comment.rs ... ok
---
[01:27:17] 
[01:27:17] error: pretty-printed source does not match expected source
[01:27:17] expected:
[01:27:17] ------------------------------------------
[01:27:17] // Check that `fn foo(x: i32, ...)` does not print as `fn foo(x: i32, ..., ...)`.
[01:27:17] // See issue #58853.
[01:27:17] 
[01:27:17] // pp-exact
[01:27:17] #![feature(c_variadic)]
[01:27:17] extern "C" {
[01:27:17]     pub fn foo(x: i32, ...);
[01:27:17] }
[01:27:17] 
[01:27:17] 
[01:27:17] pub unsafe extern "C" fn bar(_: i32, mut ap: ...) -> usize {
[01:27:17]     ap.arg::<usize>()
[01:27:17] }
[01:27:17] fn main() {}
[01:27:17] 
[01:27:17] ------------------------------------------
[01:27:17] actual:
[01:27:17] actual:
[01:27:17] ------------------------------------------
[01:27:17] // Check that `fn foo(x: i32, ...)` does not print as `fn foo(x: i32, ..., ...)`.
[01:27:17] // See issue #58853.
[01:27:17] 
[01:27:17] // pp-exact
[01:27:17] #![feature(c_variadic)]
[01:27:17] extern "C" {
[01:27:17]     pub fn foo(x: i32, ...);
[01:27:17] }
[01:27:17] 
[01:27:17] 
[01:27:17] pub unsafe extern "C" fn bar(_: i32, mut ap: ...) -> usize {
[01:27:17]     ap.arg::<usize>()
[01:27:17] }
[01:27:17] fn main() { }
[01:27:17] 
[01:27:17] ------------------------------------------
[01:27:17] 
[01:27:17] 
[01:27:17] 
[01:27:17] thread '[pretty] pretty/fn-variadic.rs' panicked at 'fatal error', src/tools/compiletest/src/runtest.rs:2040:9
[01:27:17] 
[01:27:17] 
[01:27:17] failures:
[01:27:17]     [pretty] pretty/fn-variadic.rs
[01:27:17]     [pretty] pretty/fn-variadic.rs
[01:27:17] 
[01:27:17] test result: FAILED. 52 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
[01:27:17] 
[01:27:17] 
[01:27:17] 
[01:27:17] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/pretty" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/pretty" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "pretty" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--llvm-version" "8.0.0\n" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:27:17] 
[01:27:17] 
[01:27:17] 
[01:27:17] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/test/pretty src/test/run-pass/pretty src/test/run-fail/pretty src/test/run-pass-valgrind/pretty src/test/run-pass-fulldeps/pretty src/tools/cargo src/tools/cargotest
[01:27:17] Build completed unsuccessfully in 0:00:57
[01:27:17] Makefile:50: recipe for target 'check-aux' failed
[01:27:17] make: *** [check-aux] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0fe422bf
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Sun Mar  3 02:59:25 UTC 2019
---
travis_time:end:10585105:start=1551581966658032182,finish=1551581966690589918,duration=32557736
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:07fa2440
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:329aeca0
travis_time:start:329aeca0
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0862b368
$ dmesg | grep -i kill

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)

ap.arg::<usize>()
}

fn main() { }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexreg looks like this has to be formatted as fn main() { }. I amended the last commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh really? Sorry, my bad. I'm so used to {} in other files, I thought it was actually standard.

@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 Mar 3, 2019
@alexreg
Copy link
Contributor

alexreg commented Mar 3, 2019

@bors r+ p=1

@bors
Copy link
Contributor

bors commented Mar 3, 2019

📌 Commit 379cd29 has been approved by alexreg

kennytm added a commit to kennytm/rust that referenced this pull request Mar 3, 2019
Fix C-variadic function printing

There is no longer a need to append the string `", ..."` to a functions
args as `...` is parsed as an argument and will appear in the functions
arguments.

Fixes: rust-lang#58853
bors added a commit that referenced this pull request Mar 3, 2019
Rollup of 14 pull requests

Successful merges:

 - #58730 (Have all methods of Filter and FilterMap use internal iteration)
 - #58780 (ManuallyDrop != MaybeUninit)
 - #58782 (Replace `s` with `self` in docs for str methods taking self.)
 - #58785 (allow specifying attributes for tool lints)
 - #58802 (Ensure `record_layout_for_printing()` is inlined.)
 - #58821 (Fixed a syntax error in the pin docs)
 - #58830 (tidy: deny(rust_2018_idioms))
 - #58832 (Revert switching to GCP on AppVeyor)
 - #58833 (tools/rustbook: deny(rust_2018_idioms))
 - #58835 (tools/remote-test-{client,server}: deny(rust_2018_idioms))
 - #58838 (Fix typo in Vec#resize_with documentation)
 - #58842 (Forbid duplicating Cargo as a dependency)
 - #58852 (Update toolchain to build NetBSD release)
 - #58865 (Fix C-variadic function printing)
@bors
Copy link
Contributor

bors commented Mar 3, 2019

⌛ Testing commit 379cd29 with merge f565cdd...

@bors bors merged commit 379cd29 into rust-lang:master Mar 3, 2019
@dlrobertson dlrobertson deleted the fix-varargs branch March 3, 2019 22:19
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.

None yet

4 participants