Remove `TypeSuper{Foldable,Visitable}` impls for `Region`. #110345

nnethercote · 2023-04-15T06:12:21Z

These traits exist so that folders/visitors can recurse into types of interest: binders, types, regions, predicates, and consts. But Region is non-recursive and cannot contain other types of interest, so its methods in these traits are trivial.

This commit inlines and removes those trivial methods.

r? @compiler-errors

nnethercote · 2023-04-15T06:13:45Z

I measured locally and this had no perf effect, but folding is hot so let's be extra careful:

@bors try @rust-timer queue

bors · 2023-04-15T06:13:54Z

⌛ Trying commit 9287098d020bd36bbce2251041ab5936c47a4b6e with merge 008f8262a6a96644469dffb2fc40929c7060c7d8...

bors · 2023-04-15T07:57:28Z

☀️ Try build successful - checks-actions
Build commit: 008f8262a6a96644469dffb2fc40929c7060c7d8 (008f8262a6a96644469dffb2fc40929c7060c7d8)

compiler-errors

r=me when perf comes back neutral

compiler/rustc_type_ir/src/visit.rs

rust-timer · 2023-04-15T22:53:55Z

Finished benchmarking commit (008f8262a6a96644469dffb2fc40929c7060c7d8): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

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)	5.6%	[5.6%, 5.6%]	1
Improvements ✅ (primary)	-2.8%	[-2.8%, -2.8%]	1
Improvements ✅ (secondary)	-3.5%	[-3.5%, -3.5%]	1
All ❌✅ (primary)	-2.8%	[-2.8%, -2.8%]	1

Cycles

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)	1.6%	[1.6%, 1.6%]	2
Regressions ❌ (secondary)	3.2%	[3.1%, 3.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.6%	[1.6%, 1.6%]	2

These traits exist so that folders/visitors can recurse into types of interest: binders, types, regions, predicates, and consts. But `Region` is non-recursive and cannot contain other types of interest, so its methods in these traits are trivial. This commit inlines and removes those trivial methods.

nnethercote · 2023-04-15T23:12:36Z

Perf is neutral.

@bors r=compiler-errors rollup

bors · 2023-04-15T23:12:38Z

📌 Commit 4460a1d has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-04-16T05:22:33Z

⌛ Testing commit 4460a1d with merge c15ae4af14829917dca6ea984e48400b916f05c1...

rust-log-analyzer · 2023-04-16T06:22:23Z

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

Click to see the possible cause of the failure (guessed by this bot)

failures:

---- [rustdoc] tests\rustdoc\trait-impl.rs stdout ----

error: rustdoc failed!
status: exit code: 0xc00000fd
command: PATH="C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\bin;C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0-bootstrap-tools\x86_64-pc-windows-gnu\release\deps;C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0\bin;C:\a\rust\rust\ninja;C:\a\rust\rust\mingw64\bin;C:\hostedtoolcache\windows\Python\3.11.2\x64\Scripts;C:\hostedtoolcache\windows\Python\3.11.2\x64;C:\msys64\usr\bin;C:\a\rust\rust\sccache;C:\Program Files\MongoDB\Server\5.0\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.9.3\x64;C:\cabal\bin;C:\ghcup\bin;C:\Program Files\dotnet;C:\mysql\bin;C:\Program Files\R\R-4.2.3\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.2\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.362-9\x64\bin;C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;C:\Program Files (x86)\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\\x86_64-pc-windows-gnu\\stage2\\bin\\rustdoc.exe" "-L" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2\\lib\\rustlib\\x86_64-pc-windows-gnu\\lib" "-L" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\rustdoc\\trait-impl\\auxiliary" "-o" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\rustdoc\\trait-impl" "--deny" "warnings" "C:\\a\\rust\\rust\\tests\\rustdoc\\trait-impl.rs"
stdout: none
--- stderr -------------------------------
thread 'main' has overflowed its stack



failures:
failures:
    [rustdoc] tests\rustdoc\trait-impl.rs

test result: FAILED. 601 passed; 1 failed; 6 ignored; 0 measured; 0 filtered out; finished in 107.09s

Some tests failed in compiletest suite=rustdoc mode=rustdoc host=x86_64-pc-windows-gnu target=x86_64-pc-windows-gnu
Build completed unsuccessfully in 0:49:22
make: *** [Makefile:78: ci-mingw-subset-1] Error 1

bors · 2023-04-16T06:22:47Z

💔 Test failed - checks-actions

nnethercote · 2023-04-16T10:30:41Z

Seems like a weird failure.

@bors retry

…fee1-dead Rollup of 3 pull requests Successful merges: - rust-lang#109665 (Remove `remap_env_constness` in queries) - rust-lang#110345 (Remove `TypeSuper{Foldable,Visitable}` impls for `Region`.) - rust-lang#110396 (Use lint via `lint_defs` instead of `lints`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…ion, r=compiler-errors Remove `TypeSuper{Foldable,Visitable}` impls for `Region`. These traits exist so that folders/visitors can recurse into types of interest: binders, types, regions, predicates, and consts. But `Region` is non-recursive and cannot contain other types of interest, so its methods in these traits are trivial. This commit inlines and removes those trivial methods. r? ``@compiler-errors``

rustbot assigned compiler-errors Apr 15, 2023

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. labels Apr 15, 2023

nnethercote mentioned this pull request Apr 15, 2023

Strict region folders #110312

Closed

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 15, 2023

This comment has been minimized.

Sign in to view

compiler-errors approved these changes Apr 15, 2023

View reviewed changes

compiler/rustc_type_ir/src/visit.rs Outdated Show resolved Hide resolved

compiler-errors 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 Apr 15, 2023

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 15, 2023

nnethercote force-pushed the rm-Super-impls-for-Region branch from 9287098 to 4460a1d Compare April 15, 2023 23:12

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 Apr 15, 2023

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 Apr 16, 2023

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 Apr 16, 2023

fee1-dead mentioned this pull request Apr 16, 2023

Rollup of 3 pull requests #110401

Merged

fee1-dead mentioned this pull request Apr 16, 2023

Rollup of 8 pull requests #110408

Closed

bors merged commit eba4191 into rust-lang:master Apr 16, 2023

rustbot added this to the 1.71.0 milestone Apr 16, 2023

nnethercote deleted the rm-Super-impls-for-Region branch April 16, 2023 21:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `TypeSuper{Foldable,Visitable}` impls for `Region`. #110345

Remove `TypeSuper{Foldable,Visitable}` impls for `Region`. #110345

nnethercote commented Apr 15, 2023

nnethercote commented Apr 15, 2023

This comment has been minimized.

bors commented Apr 15, 2023

bors commented Apr 15, 2023

This comment has been minimized.

compiler-errors left a comment

rust-timer commented Apr 15, 2023

nnethercote commented Apr 15, 2023

bors commented Apr 15, 2023

bors commented Apr 16, 2023

rust-log-analyzer commented Apr 16, 2023

bors commented Apr 16, 2023

nnethercote commented Apr 16, 2023

Remove TypeSuper{Foldable,Visitable} impls for Region. #110345

Remove TypeSuper{Foldable,Visitable} impls for Region. #110345

Conversation

nnethercote commented Apr 15, 2023

nnethercote commented Apr 15, 2023

This comment has been minimized.

bors commented Apr 15, 2023

bors commented Apr 15, 2023

This comment has been minimized.

compiler-errors left a comment

Choose a reason for hiding this comment

rust-timer commented Apr 15, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

nnethercote commented Apr 15, 2023

bors commented Apr 15, 2023

bors commented Apr 16, 2023

rust-log-analyzer commented Apr 16, 2023

bors commented Apr 16, 2023

nnethercote commented Apr 16, 2023

Remove `TypeSuper{Foldable,Visitable}` impls for `Region`. #110345

Remove `TypeSuper{Foldable,Visitable}` impls for `Region`. #110345