Skip to content

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Oct 10, 2023

Follow-up of #116142.

It was mentioned in the team meeting that an enum with a repr should also get its discriminants displayed. Forgot to implement it in #116142...

It also allowed to uncover a bug: i was not providing the correct DefId in case it was a type alias to render_enum_fields. It's also fixed in this PR.

r? @fmease

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Oct 10, 2023
Comment on lines +1253 to +1254
let ty = cx.tcx().type_of(it.def_id().unwrap()).instantiate_identity();
let enum_def_id = ty.ty_adt_id().unwrap();
Copy link
Member

@fmease fmease Oct 11, 2023

Choose a reason for hiding this comment

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

I wish we didn't need to do this and could just store the DefId of the inner item inside TypeAliasInnerType and obtain it directly via adt.did() in clean_ty_alias_inner_type but that would increase the size of the type ... so your code is fine.

Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Oct 11, 2023

Choose a reason for hiding this comment

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

Unfortunately yes. We could maybe add a helper method later on if we need to do it once again though.

@fmease
Copy link
Member

fmease commented Oct 11, 2023

r=me with nits addressed unless you'd like to fcp this (which I don't think is strictly necessary since we've briefly talked about it in the meeting).

@GuillaumeGomez GuillaumeGomez force-pushed the repr-enums-discriminant branch from ff7612c to 00c3de9 Compare October 11, 2023 21:46
@GuillaumeGomez
Copy link
Member Author

It was confirmed in the meeting with the rest so I think it's fine to go along. It should have been part of the previous PR, just completely forgot about it...

@fmease
Copy link
Member

fmease commented Oct 11, 2023

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 11, 2023

📌 Commit 00c3de9 has been approved by fmease

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 Oct 11, 2023
@bors
Copy link
Collaborator

bors commented Oct 12, 2023

⌛ Testing commit 00c3de9 with merge 8e3daa6...

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2023
…nt, r=fmease

Show enum discriminant if a compatible repr is used

Follow-up of rust-lang#116142.

It was mentioned in the [team meeting](https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.2002-10-2023/near/394504024) that an enum with a `repr` should also get its discriminants displayed. Forgot to implement it in rust-lang#116142...

It also allowed to uncover a bug: i was not providing the correct `DefId` in case it was a type alias to `render_enum_fields`. It's also fixed in this PR.

r? `@fmease`
@rust-log-analyzer
Copy link
Collaborator

The job i686-msvc failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
---- [codegen-units] tests\codegen-units\partitioning\local-inlining-but-not-all.rs stdout ----

error: compilation failed!
status: exit code: 1
command: PATH="C:\a\rust\rust\build\i686-pc-windows-msvc\stage2\bin;C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64;C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64;C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x86;C:\a\rust\rust\build\i686-pc-windows-msvc\stage0-bootstrap-tools\i686-pc-windows-msvc\release\deps;C:\a\rust\rust\build\i686-pc-windows-msvc\stage0\bin;C:\Program Files\PowerShell\7;C:\a\rust\rust\ninja;C:\a\rust\rust\msys2\mingw32\bin;C:\hostedtoolcache\windows\Python\3.11.5\x64\Scripts;C:\hostedtoolcache\windows\Python\3.11.5\x64;C:\msys64\usr\bin;C:\a\rust\rust\sccache;C:\PROGRA~1\MongoDB\bin;C:\aliyun-cli;C:\vcpkg;C:\cf-cli;C:\Program Files (x86)\NSIS;C:\tools\zstd;C:\Program Files\Mercurial;C:\hostedtoolcache\windows\stack\2.11.1\x64;C:\cabal\bin;C:\ghcup\bin;C:\Program Files\dotnet;C:\mysql\bin;C:\Program Files\R\R-4.3.1\bin\x64;C:\SeleniumWebDrivers\GeckoDriver;C:\Program Files (x86)\sbt\bin;C:\Program Files (x86)\GitHub CLI;C:\Program Files\Git\bin;C:\Program Files (x86)\pipx_bin;C:\npm\prefix;C:\hostedtoolcache\windows\go\1.20.8\x64\bin;C:\hostedtoolcache\windows\Python\3.7.9\x64\Scripts;C:\hostedtoolcache\windows\Python\3.7.9\x64;C:\hostedtoolcache\windows\Ruby\2.5.9\x64\bin;C:\Program Files\OpenSSL\bin;C:\tools\kotlinc\bin;C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.382-5\x64\bin;C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;C:\ProgramData\kind;C:\Program Files\Eclipse Foundation\jdk-8.0.302.8-hotspot\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\ProgramData\Chocolatey\bin;C:\Program Files\PowerShell\7;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit;C:\Program Files (x86)\Microsoft SQL Server\110\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\130\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\140\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\150\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\160\DTS\Binn;C:\Strawberry\c\bin;C:\Strawberry\perl\site\bin;C:\Strawberry\perl\bin;C:\ProgramData\chocolatey\lib\pulumi\tools\Pulumi\bin;C:\Program Files\TortoiseSVN\bin;C:\Program Files\CMake\bin;C:\ProgramData\chocolatey\lib\maven\apache-maven-3.8.7\bin;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\Program Files\nodejs;C:\Program Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Program Files\GitHub CLI;C:\tools\php;C:\Program Files (x86)\sbt\bin;C:\SeleniumWebDrivers\ChromeDriver;C:\SeleniumWebDrivers\EdgeDriver;C:\Program Files\Amazon\AWSCLIV2;C:\Program Files\Amazon\SessionManagerPlugin\bin;C:\Program Files\Amazon\AWSSAMCLI\bin;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files (x86)\Microsoft BizTalk Server;C:\Program Files\LLVM\bin;C:\Users\runneradmin\.dotnet\tools;C:\Users\runneradmin\.cargo\bin;C:\Users\runneradmin\AppData\Local\Microsoft\WindowsApps" "C:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\bin\\rustc.exe" "C:\\a\\rust\\rust\\tests\\codegen-units\\partitioning\\local-inlining-but-not-all.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=C:\\Users\\runneradmin\\.cargo" "--sysroot" "C:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2" "--target=i686-pc-windows-msvc" "-C" "incremental=C:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\test\\codegen-units\\partitioning\\local-inlining-but-not-all\\local-inlining-but-not-all.inc" "-Z" "incremental-verify-ich" "-Z" "human_readable_cgu_names" "-O" "-C" "prefer-dynamic" "--out-dir" "C:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\test\\codegen-units\\partitioning\\local-inlining-but-not-all" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=C:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\native\\rust-test-helpers" "-L" "C:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\test\\codegen-units\\partitioning\\local-inlining-but-not-all\\auxiliary" "-Zprint-mono-items=lazy" "-Zinline-in-all-cgus=no"
--- stdout -------------------------------
MONO_ITEM fn inline::inlined_function @@ local_inlining_but_not_all.96b6a1c021725c50-inline[External]
MONO_ITEM fn non_user::baz @@ local_inlining_but_not_all.96b6a1c021725c50-non_user[External]
MONO_ITEM fn user1::foo @@ local_inlining_but_not_all.96b6a1c021725c50-user1[External]
MONO_ITEM fn user2::bar @@ local_inlining_but_not_all.96b6a1c021725c50-user2[External]
--- stderr -------------------------------
--- stderr -------------------------------
error: couldn't create a temp dir: Access is denied. (os error 5) at path "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\rustcA6qfxU"
error: aborting due to previous error
------------------------------------------


---
test result: FAILED. 40 passed; 1 failed; 3 ignored; 0 measured; 0 filtered out; finished in 1.03s

Some tests failed in compiletest suite=codegen-units mode=codegen-units host=i686-pc-windows-msvc target=i686-pc-windows-msvc
Build completed unsuccessfully in 0:41:22
make: *** [Makefile:73: ci-msvc-ps1] Error 1
  network time: Thu, 12 Oct 2023 05:21:02 GMT
##[error]Process completed with exit code 2.
Post job cleanup.
[command]"C:\Program Files\Git\bin\git.exe" version

@bors
Copy link
Collaborator

bors commented Oct 12, 2023

💔 Test failed - checks-actions

@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 Oct 12, 2023
@GuillaumeGomez
Copy link
Member Author

@bors retry

@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 Oct 12, 2023
@bors
Copy link
Collaborator

bors commented Oct 12, 2023

⌛ Testing commit 00c3de9 with merge 3ff244b...

@bors
Copy link
Collaborator

bors commented Oct 12, 2023

☀️ Test successful - checks-actions
Approved by: fmease
Pushing 3ff244b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 12, 2023
@bors bors merged commit 3ff244b into rust-lang:master Oct 12, 2023
@rustbot rustbot added this to the 1.75.0 milestone Oct 12, 2023
@GuillaumeGomez GuillaumeGomez deleted the repr-enums-discriminant branch October 12, 2023 12:32
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3ff244b): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.4% [-0.4%, -0.4%] 3
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.2% [1.5%, 3.0%] 2
Improvements ✅
(primary)
-6.0% [-6.0%, -6.0%] 1
Improvements ✅
(secondary)
-3.5% [-3.9%, -2.9%] 3
All ❌✅ (primary) -6.0% [-6.0%, -6.0%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 626.317s -> 627.587s (0.20%)
Artifact size: 271.25 MiB -> 271.32 MiB (0.02%)

bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Oct 13, 2023
50: Automated pull from upstream `master` r=Dajamante a=github-actions[bot]


This PR pulls the following changes from the upstream repository:

* rust-lang/rust#116619
* rust-lang/rust#115964
* rust-lang/rust#116391
* rust-lang/rust#116510
* rust-lang/rust#116671
  * rust-lang/rust#116669
  * rust-lang/rust#116654
  * rust-lang/rust#116642
  * rust-lang/rust#116625
  * rust-lang/rust#116593
* rust-lang/rust#116649
* rust-lang/rust#116600
* rust-lang/rust#116628



Co-authored-by: Nadrieril <[email protected]>
Co-authored-by: Scott McMurray <[email protected]>
Co-authored-by: bjorn3 <[email protected]>
Co-authored-by: Nicholas Nethercote <[email protected]>
Co-authored-by: Trevor Gross <[email protected]>
Co-authored-by: Georg Semmler <[email protected]>
Co-authored-by: Guillaume Gomez <[email protected]>
Co-authored-by: Gurinder Singh <[email protected]>
Co-authored-by: bors <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

6 participants