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

Stabilize version output for rustc and rustdoc #13816

Closed
wants to merge 1 commit into from
Closed

Stabilize version output for rustc and rustdoc #13816

wants to merge 1 commit into from

Conversation

anemator
Copy link
Contributor

Fixes #13582

if matches.opt_present("version") {
match matches.opt_str("version").as_ref().map(|s| s.as_slice()) {
None
| Some("short") => version(binary, VdShort),
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't need a short option. I think I'd prefer just matching on Some("verbose") and None. You can pass a bool to version function.

Copy link
Contributor

Choose a reason for hiding this comment

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

actually, given that this match is duplicated in rustdoc. What about moving it into the version function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Works for me.

@flaper87
Copy link
Contributor

Thanks for stabbing this. Could you please add a run-make test for this? Thanks

@anemator
Copy link
Contributor Author

Sure thing. Thanks for your help.

Any idea why the build failed? I ran a make check prior to committing.

@alexcrichton
Copy link
Member

The travis failure looks like a fluke. The travis boxes often don't have quite enough memory to run the test suite, and it looks like that build suffered from a similar problem.

@flaper87
Copy link
Contributor

I think @alexcrichton closed this PR accidentally.

@flaper87 flaper87 reopened this Apr 29, 2014
@alexcrichton
Copy link
Member

Oops, sorry! I didn't even realize...

@anemator
Copy link
Contributor Author

No problem. Thanks for reopening @flaper87 .

@anemator
Copy link
Contributor Author

anemator commented May 8, 2014

This updated patch includes @pnkfelix suggestion (partially) of outputting debug and optimize flags, but it only does that for the rustc crate. I couldn't find a good alternative to include those flags for all the crates, the only decent option I saw was to embed the flags in each crate, then include each one directly in librustc....but I couldn't think of a reason to mix crate flags. Is that a common scenario?

Also, running rustdoc --version bad_verbose_arg outputs task '<main>' failed at 'Box<Any>'. I'm not sure of the reason, but it seems that calling (indirectly) librustc::driver::early_error from outside of librustc (librustdoc specifically) triggers it.

@brson
Copy link
Contributor

brson commented May 12, 2014

@anemator Thanks for working on this.

Can you change the way BUILD_FLAGS works? I think this method of creating BUILD_FLAGS in the Makefiles is error prone, unlikely to capture all the flags passed on the command line. A better way to do this might be to have something like a buildcmd! macro. That said though, I would be more comfortable simply dropping that functionality for now, since adding a built-in macro is a big change.

@alexcrichton
Copy link
Member

This may also wish to include #14162 in that argv[0] isn't printed, but rather just "rustc" or "rustdoc"

@brson
Copy link
Contributor

brson commented May 14, 2014

Oh, yeah. Replacing argv[0] is the one thing we need to do.

@brson
Copy link
Contributor

brson commented May 14, 2014

I went ahead and reopened and approved #14162.

@anemator
Copy link
Contributor Author

@brson Thanks for the tip on adding a macro.

I chose to do it with the BUILD_FLAGS variable initially to avoid potentially inserting user info into the binary via CFG_ARGS and to be certain of the builder's debug and optimize flags since they can be enabled in multiple ways (not necessarily via CFG_ARGS).

However, I removed BUILD_FLAGS for now since as mentioned, it may be flaky and there also seem to be differing opinions as to what should be included.

@brson
Copy link
Contributor

brson commented Jun 12, 2014

Sorry for the delay.

@anemator
Copy link
Contributor Author

No problem.

For reference, the normal format is

rustc 0.11.0-pre (77453ac8511c2538ba299198f48268402186ce7e 2014-06-14 12:40:48 -0400)

and the verbose is

rustc 0.11.0-pre (77453ac8511c2538ba299198f48268402186ce7e 2014-06-14 12:40:48 -0400)
binary: rustc
commit-hash: 77453ac8511c2538ba299198f48268402186ce7e
commit-date: 2014-06-14 12:40:48 -0400
host: x86_64-apple-darwin
release: 0.11.0-pre

@flaper87
Copy link
Contributor

@brson @alexcrichton can this patch be approved? Is it blocked by something else?

@alexcrichton
Copy link
Member

I canceled the build to get the rollup running (this PR is in the rollup). I have also marked this PR with @bors: retry so if the rollup fails it'll be retried.

arcnmx pushed a commit to arcnmx/rust that referenced this pull request Jan 9, 2023
…, r=Veykril

Postfix adjustment hints

# Basic Description

This PR implements "postfix" adjustment hints:
![2022-12-21_19-27](https://user-images.githubusercontent.com/38225716/208941721-d48d316f-a918-408a-9757-8d4e2b402a66.png)

They are identical to normal adjustment hints, but are rendered _after_ the expression. E.g. `expr.*` instead of `*expr`. ~~This mirrors "postfix deref" feature that I'm planning to eventually propose to the compiler.~~

# Motivation

The advantage of being postfix is that you need to add parentheses less often:

![2022-12-21_19-38](https://user-images.githubusercontent.com/38225716/208944302-16718112-14a4-4438-8aed-797766391c63.png)
![2022-12-21_19-37](https://user-images.githubusercontent.com/38225716/208944281-d9614888-6597-41ee-bf5d-a081d8048f94.png)

This is because a lot of "reborrow" hints are caused by field access or method calls, both of which are postfix and have higher "precedence" than prefix `&` and `*`.

Also IMHO it just looks nicer and it's more clear what is happening (order of operations).

# Modes

However, there are some cases where postfix hints need parentheses but prefix don't (for example `&x` being turned into `(&x).*.*.&` or `&**&x`).

This PR allows users to choose which look they like more. There are 4 options (`rust-analyzer.inlayHints.expressionAdjustmentHints.mode` setting):
- `prefix` — always use prefix hints (default, what was used before that PR)
- `postfix` — always use postfix hints
- `prefer_prefix` — try to minimize number of parentheses, breaking ties in favor of prefix
- `prefer_postfix` — try to minimize number of parentheses, breaking ties in favor of postfix

Comparison of all modes:

![2022-12-21_19-53](https://user-images.githubusercontent.com/38225716/208947482-26357c82-2b42-47d9-acec-835f5f03f6b4.png)
![2022-12-21_19-49](https://user-images.githubusercontent.com/38225716/208946731-fe566d3b-52b2-4846-994d-c2cecc769e0f.png)
![2022-12-21_19-48](https://user-images.githubusercontent.com/38225716/208946742-6e237f44-805e-469a-a3db-03d8f76e1317.png)
![2022-12-21_19-47](https://user-images.githubusercontent.com/38225716/208946747-79f25fae-e3ea-47d2-8d27-cb4eeac034fe.png)

# Edge cases

Where are some rare cases where chain hints weirdly interact with adjustment hints, for example (note `SourceAnalyzer.&`):

![image](https://user-images.githubusercontent.com/38225716/208947958-41c12971-f1f0-4a41-a930-47939cce9f58.png)

This is pre-existing, you can get the same effect with prefix hints (`SourceAnalyzer)`).

----

Another weird thing is this:

![2022-12-21_20-00](https://user-images.githubusercontent.com/38225716/208948590-ea26d325-2108-4b35-abaa-716a65a1ae99.png)

Here `.&` is a hint and `?` is written in the source code. It looks like `?` is part of the hint because `?.` is ligature in my font. IMO this is a bug in vscode, but still worth mentioning (I'm also too lazy to report it there...).

# Fixed bugs

I've used the "needs parens" API and this accidentally fixed a bug with parens around `as`, see the test diff:
```diff,rust
     let _: *const u32  = &mut 0u32 as *mut u32;
                        //^^^^^^^^^^^^^^^^^^^^^<mut-ptr-to-const-ptr>
+                       //^^^^^^^^^^^^^^^^^^^^^(
+                       //^^^^^^^^^^^^^^^^^^^^^)
...
     let _: *const u32  = &mut 0u32 as *mut u32;
                        //^^^^^^^^^^^^^^^^^^^^^<mut-ptr-to-const-ptr>
+                       //^^^^^^^^^^^^^^^^^^^^^(
+                       //^^^^^^^^^^^^^^^^^^^^^)
```

# Changelog

changelog feature Add an option to make adjustment hints (aka reborrow hints) postfix
changelog fix Fix placement of parentheses around `as` casts for adjustment hints
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.

Consider --version output more carefully
5 participants