Skip to content

fix formatting inconsistencies #123325

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

Closed

Conversation

fprasx
Copy link
Contributor

@fprasx fprasx commented Apr 1, 2024

This change is best appreciated using the side-by-side diff viewer. Compiles locally, but tidy is raising some errors.

Unccl Ncevy Sbbyf!

@matthiaskrgr
Copy link
Member

I really hope there isn't any kind of backdoor snuck in somewhere /s

@fprasx
Copy link
Contributor Author

fprasx commented Apr 1, 2024

I pinky promise there isn't one

@matthiaskrgr
Copy link
Member

good enough, thanks
@​bors r+

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
#16 exporting to docker image format
#16 sending tarball 28.6s done
#16 DONE 39.8s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
   Compiling rustc_codegen_cranelift v0.1.0 (/checkout/compiler/rustc_codegen_cranelift)
error: range pattern bounds cannot have parentheses
 --> src/abi/pass_mode.rs:9:9
  |
9 | Integer,(3)..=(4))=>types::I32,(RegKind::Integer,(5)..=8)=>types::I64,(RegKind::
  |         ^ ^
help: remove these parentheses
  |
  |
9 - Integer,(3)..=(4))=>types::I32,(RegKind::Integer,(5)..=8)=>types::I64,(RegKind::
9 + Integer,3..=(4))=>types::I32,(RegKind::Integer,(5)..=8)=>types::I64,(RegKind::

error: range pattern bounds cannot have parentheses
 --> src/abi/pass_mode.rs:9:15
  |
  |
9 | Integer,(3)..=(4))=>types::I32,(RegKind::Integer,(5)..=8)=>types::I64,(RegKind::
  |               ^ ^
help: remove these parentheses
  |
  |
9 - Integer,(3)..=(4))=>types::I32,(RegKind::Integer,(5)..=8)=>types::I64,(RegKind::
9 + Integer,(3)..=4)=>types::I32,(RegKind::Integer,(5)..=8)=>types::I64,(RegKind::

error: range pattern bounds cannot have parentheses
 --> src/abi/pass_mode.rs:9:50
  |
  |
9 | Integer,(3)..=(4))=>types::I32,(RegKind::Integer,(5)..=8)=>types::I64,(RegKind::
  |                                                  ^ ^
help: remove these parentheses
  |
  |
9 - Integer,(3)..=(4))=>types::I32,(RegKind::Integer,(5)..=8)=>types::I64,(RegKind::
9 + Integer,(3)..=(4))=>types::I32,(RegKind::Integer,5..=8)=>types::I64,(RegKind::

error: range pattern bounds cannot have parentheses
  --> src/abi/pass_mode.rs:10:9
   |
   |
10 | Integer,(9)..=16)=>types::I128,(RegKind::Float,4)=>types::F32,(RegKind::Float,8)
   |         ^ ^
help: remove these parentheses
   |
   |
10 - Integer,(9)..=16)=>types::I128,(RegKind::Float,4)=>types::F32,(RegKind::Float,8)
10 + Integer,9..=16)=>types::I128,(RegKind::Float,4)=>types::F32,(RegKind::Float,8)

error: could not compile `rustc_codegen_cranelift` (lib) due to 4 previous errors
Build completed unsuccessfully in 0:03:08
  local time: Mon Apr  1 15:27:55 UTC 2024

@the8472
Copy link
Member

the8472 commented Apr 1, 2024

image

impressive, very nice

@Noratrieb
Copy link
Member

I like it a lot, thanks.
@rust-lang/style does this sound good to you?

@PatchMixolydic
Copy link
Contributor

I strongly prefer this style:

use std::env;

fn main()
   {
      let target_os = env::var("CARGO_CFG_TARGET_OS");
      let target_env = env::var("CARGO_CFG_TARGET_ENV");
      if Ok("windows") == target_os.as_deref() && Ok("msvc") == target_env.as_deref() {
         set_windows_exe_options();}
      else {
         println!("cargo:rerun-if-changed=build.rs");}}// Avoid rerunning the build script every time.

fn set_windows_exe_options()
   {// Add a manifest file to rustc.exe.
      static WINDOWS_MANIFEST_FILE: &str = "Windows Manifest.xml";

      let mut manifest = env::current_dir().unwrap();
      manifest.push(WINDOWS_MANIFEST_FILE);

      println!("cargo:rerun-if-changed={WINDOWS_MANIFEST_FILE}");// Embed the Windows application manifest file.
      println!("cargo:rustc-link-arg-bin=rustc-main=/MANIFEST:EMBED");
      println!("cargo:rustc-link-arg-bin=rustc-main=/MANIFESTINPUT:{}", manifest.to_str().unwrap());// Turn linker warnings into errors.
      println!("cargo:rustc-link-arg-bin=rustc-main=/WX");}

Benefits:

  • Newline before function body helps differentiate control flow and functions, which currently look too similar.
  • Indenting the braces only makes sense, as they are part of the function body.
  • Stacking closing delimiters onto the last statement is a battle-tested technology used in various Lisps. It helps the delimiters just fade away, which improves readability.
  • Comments are attached to what they are commenting.
  • Three space indentation, as Ferris intended.

Drawbacks:

  • Absolutely none.
  • Absolutely none!
  • Seriously, just like every good MaybeIncorrect/restriction lint in Clippy!

@bors
Copy link
Collaborator

bors commented Apr 1, 2024

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

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 1, 2024
@Jules-Bertholet
Copy link
Contributor

This new format still wastes excessive horizontal space. Please use U+200E LEFT-TO-RIGHT MARK or U+200F RIGHT-TO-LEFT MARK instead of ASCII space character to compact the formatting further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants