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

Rollup of 6 pull requests #127952

Closed
wants to merge 21 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

veera-sivarajan and others added 21 commits July 4, 2024 19:16
Previously we would only mention that the item was gated out, and opportunisitically mention the feature flag name when possible. We now point to the place where the item was gated, which can be behind layers of macro indirection, or in different modules.

```
error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner`
  --> $DIR/diagnostics-cross-crate.rs:18:23
   |
LL |     cfged_out::inner::doesnt_exist::hello();
   |                       ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner`
   |
note: found an item that was configured out
  --> $DIR/auxiliary/cfged_out.rs:6:13
   |
LL |     pub mod doesnt_exist {
   |             ^^^^^^^^^^^^
note: the item is gated here
  --> $DIR/auxiliary/cfged_out.rs:5:5
   |
LL |     #[cfg(FALSE)]
   |     ^^^^^^^^^^^^^
```
The `Option`s within the `ReplaceRange`s within the hashmap are always
`None`. This PR omits them and inserts them when they are extracted from
the hashmap.
It's no shorter than `ret.attrs()`, and `ret.attrs()` is used multiple
times earlier in the function.
This puts it just before the `replace_ranges` initialization, which
makes sense because the two variables are closely related.
Currently in `collect_tokens_trailing_token`, `start_pos` and `end_pos`
are 1-indexed by `replace_ranges` is 0-indexed, which is really
confusing. Making them both 0-indexed makes debugging much easier.
No need to collect tokens on this recovery path, because the parsed
statement isn't even looked at.
Use a parameter to decide whether to force collect, as is done for the
closely related `parse_local_mk` method.
Instead of a `bool`. Because `ForceCollect` is used in this way
everywhere else.
Adding details, clarifying lots of little things, etc. In particular,
the commit adds details of an example. I find this very helpful, because
it's taken me a long time to understand how this code works.
There are three places where we currently check `force_collect` and call
`collect_tokens_no_attrs` for `ForceCollect::Yes` and a vanilla parsing
function for `ForceCollect::No`.

But we can instead just pass in `force_collect` and let
`collect_tokens_trailing_token` do the appropriate thing.
Parser: Suggest Placing the Return Type After Function Parameters

Fixes rust-lang#126311

This PR suggests placing the return type after the function parameters when it's misplaced after a `where` clause.

This also tangentially improves diagnostics for cases like [this](https://github.com/veera-sivarajan/rust/blob/86d6f1312a77997ef994240e716288d61a343a6d/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs#L1C1-L1C28) and adds doc comments for `parser::AllowPlus`.
When finding item gated behind a `cfg` flag, point at it

Previously we would only mention that the item was gated out, and opportunisitically mention the feature flag name when possible. We now point to the place where the item was gated, which can be behind layers of macro indirection, or in different modules.

```
error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner`
  --> $DIR/diagnostics-cross-crate.rs:18:23
   |
LL |     cfged_out::inner::doesnt_exist::hello();
   |                       ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner`
   |
note: found an item that was configured out
  --> $DIR/auxiliary/cfged_out.rs:6:13
   |
LL |     pub mod doesnt_exist {
   |             ^^^^^^^^^^^^
note: the item is gated here
  --> $DIR/auxiliary/cfged_out.rs:5:5
   |
LL |     #[cfg(FALSE)]
   |     ^^^^^^^^^^^^^
```
…g_token-cleanups, r=petrochenkov

`collect_tokens_trailing_token` cleanups

More cleanups I made while understanding the code for processing `cfg_attr`, to fix test failures in rust-lang#124141.

r? ``@petrochenkov``
…ents, r=petrochenkov

`force_collect` improvements

Yet more cleanups relating to `cfg_attr` processing.

r? ``@petrochenkov``
…orino

Don't allow unsafe statics outside of extern blocks

This PR fixes a regression where we allowed `unsafe static` items in top-level modules (i.e. outside of `unsafe extern` blocks).

It's harder IMO to integrate this into the `check_item_safety` function, so I opted to just put this check on the `static` item itself.

Beta version of this lives at rust-lang#127944.

r? `@oli-obk` or `@spastorino`
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jul 19, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Jul 19, 2024

📌 Commit 11d0acc has been approved by matthiaskrgr

It is now in the queue for this repository.

@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 Jul 19, 2024
@matthiaskrgr
Copy link
Member Author

@bors rollup=never

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Getting action download info
Download action repository 'msys2/setup-msys2@v2.22.0' (SHA:cc11e9188b693c2b100158c3322424c4cc1dadea)
Download action repository 'actions/checkout@v4' (SHA:692973e3d937129bcbf40652eb9f2f61becf3332)
Download action repository 'actions/upload-artifact@v4' (SHA:0b2256b8c012f0828dc542b3febcab082c67f72b)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt \
    && pip3 install virtualenv
COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/

# NOTE: intentionally uses python2 for x.py so we can test it still works.
# NOTE: intentionally uses python2 for x.py so we can test it still works.
# validate-toolstate only runs in our CI, so it's ok for it to only support python3.
ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \
           --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --allow-unsafe --generate-hashes reuse-requirements.in
---

#12 [5/8] COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
#12 DONE 0.0s

#13 [6/8] RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt     && pip3 install virtualenv
#13 0.455   Downloading binaryornot-0.4.4-py2.py3-none-any.whl (9.0 kB)
#13 0.473 Collecting boolean-py==4.0
#13 0.481   Downloading boolean.py-4.0-py3-none-any.whl (25 kB)
#13 0.497 Collecting chardet==5.1.0
---
#13 4.063 Building wheels for collected packages: reuse
#13 4.064   Building wheel for reuse (pyproject.toml): started
#13 4.405   Building wheel for reuse (pyproject.toml): finished with status 'done'
#13 4.406   Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=181117 sha256=f5f58750481f69515c2c0d1d503daf565e2565c370d07fc6aeb95fe3498b4269
#13 4.407   Stored in directory: /tmp/pip-ephem-wheel-cache-00hkqawd/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d
#13 4.409 Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet
#13 4.432   Attempting uninstall: setuptools
#13 4.433     Found existing installation: setuptools 59.6.0
#13 4.434     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
#13 4.434     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
#13 4.435     Can't uninstall 'setuptools'. No files were found to uninstall.
#13 5.143 Successfully installed binaryornot-0.4.4 boolean-py-4.0 chardet-5.1.0 jinja2-3.1.2 license-expression-30.0.0 markupsafe-2.1.1 python-debian-0.1.49 reuse-1.1.0 setuptools-66.0.0
#13 5.144 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#13 5.696 Collecting virtualenv
#13 5.744   Downloading virtualenv-20.26.3-py3-none-any.whl (5.7 MB)
#13 5.969      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 5.7/5.7 MB 25.5 MB/s eta 0:00:00
#13 6.012 Collecting distlib<1,>=0.3.7
#13 6.019   Downloading distlib-0.3.8-py2.py3-none-any.whl (468 kB)
#13 6.066 Collecting filelock<4,>=3.12.2
#13 6.074   Downloading filelock-3.15.4-py3-none-any.whl (16 kB)
#13 6.074   Downloading filelock-3.15.4-py3-none-any.whl (16 kB)
#13 6.106 Collecting platformdirs<5,>=3.9.1
#13 6.113   Downloading platformdirs-4.2.2-py3-none-any.whl (18 kB)
#13 6.204 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#13 6.392 Successfully installed distlib-0.3.8 filelock-3.15.4 platformdirs-4.2.2 virtualenv-20.26.3
#13 DONE 6.5s

#14 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#14 DONE 0.0s
---
DirectMap4k:      210880 kB
DirectMap2M:     7129088 kB
DirectMap1G:    11534336 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
downloading https://ci-artifacts.rust-lang.org/rustc-builds-alt/3d68afc9e821b00d59058abc9bda670b07639955/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz
extracting /checkout/obj/build/cache/llvm-3d68afc9e821b00d59058abc9bda670b07639955-true/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz to /checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm
---
fmt check
fmt: checked 5325 files
tidy check
tidy: Skipping binary file check, read-only filesystem
##[error]tidy error: /checkout/compiler/rustc_parse/src/parser/attr_wrapper.rs:186: unexplained "```ignore" doctest; try one:
* make the test actually pass, by adding necessary imports and declarations, or
* make the test actually pass, by adding necessary imports and declarations, or
* use "```text", if the code is not Rust code, or
* use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or
* use "```should_panic", if the code is expected to fail at run time, or
* use "```no_run", if the code should type-check but not necessary linkable/runnable, or
* explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided.

removing old virtual environment
removing old virtual environment
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10'
Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (24.1)
  Downloading pip-24.1.2-py3-none-any.whl.metadata (3.6 kB)
Downloading pip-24.1.2-py3-none-any.whl (1.8 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.8/1.8 MB 43.3 MB/s eta 0:00:00
Installing collected packages: pip
---
All checks passed!
checking C++ file formatting
some tidy checks failed

Command LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib" RUSTC="/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/rustc" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/rust-tidy" "/checkout" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "/checkout/obj/build" "4" "--extra-checks=py:lint,cpp:fmt" (failure_mode=DelayFail, stdout_mode=Print, stderr_mode=Print) did not execute successfully.
Created at: src/core/build_steps/tool.rs:1062:23
Executed at: src/core/build_steps/test.rs:1084:29

Build completed unsuccessfully in 0:01:18

@tgross35
Copy link
Contributor

🤔 where did that tidy lint come from

@tgross35
Copy link
Contributor

#127902 seems to be the one getting flagged

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 19, 2024
@tgross35 tgross35 closed this Jul 19, 2024
@matthiaskrgr matthiaskrgr deleted the rollup-jd35ci0 branch September 1, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants