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

Enforce well formedness for type alias impl trait's hidden type #95519

Merged
merged 2 commits into from
Apr 8, 2022

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Mar 31, 2022

fixes #84657

This was not an issue with return-position-impl-trait because the generic bounds of the function are the same as those of the opaque type, and the hidden type must already be well formed within the function.

With type-alias-impl-trait the hidden type could be defined in a function that has more lifetime bounds than the type alias. This is fine, but the hidden type must still be well formed without those additional bounds.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 31, 2022
@rust-highfive
Copy link
Collaborator

r? @cjgillot

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 31, 2022
@rust-log-analyzer

This comment has been minimized.

@cjgillot cjgillot 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 Mar 31, 2022
@oli-obk oli-obk added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 1, 2022
@rust-log-analyzer

This comment has been minimized.

@oli-obk oli-obk 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 1, 2022
@oli-obk oli-obk added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 6, 2022
@@ -144,7 +144,7 @@ impl<'a, K, V> IntoIterator for &'a VecMap<K, V> {
}
}

impl<'a, K, V> IntoIterator for &'a mut VecMap<K, V> {
impl<'a, K: 'a, V: 'a> IntoIterator for &'a mut VecMap<K, V> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason this is required is that associated type alias impl trait does not imply bounds from the type that the impl is for.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be concerned for breakage and get a lang team signoff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, this is just a nightly feature. We can add more implicit magic later

Copy link
Member

Choose a reason for hiding this comment

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

yeah, i think this is something we can fix later

@cjgillot
Copy link
Contributor

cjgillot commented Apr 7, 2022

I am probably not the right reviewer for type-related stuff.
r? @compiler-errors

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

loooks good, and it's behind a feature-gate anyways so i'm not worried about the regression in associated TAIT.

@bors r+

@@ -144,7 +144,7 @@ impl<'a, K, V> IntoIterator for &'a VecMap<K, V> {
}
}

impl<'a, K, V> IntoIterator for &'a mut VecMap<K, V> {
impl<'a, K: 'a, V: 'a> IntoIterator for &'a mut VecMap<K, V> {
Copy link
Member

Choose a reason for hiding this comment

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

yeah, i think this is something we can fix later

@compiler-errors
Copy link
Member

oh lol bors does not respond to approval messages it seems

@bors r+
@bors rollup=never (so we can bisect this better later, feel free to change rollup status as you see fit)

@bors
Copy link
Contributor

bors commented Apr 7, 2022

📌 Commit b30bcfa has been approved by compiler-errors

@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 Apr 7, 2022
@bors
Copy link
Contributor

bors commented Apr 8, 2022

⌛ Testing commit b30bcfa with merge b13b9f84bef48e5857378c07094fa59e8986f76c...

@bors
Copy link
Contributor

bors commented Apr 8, 2022

💔 Test failed - checks-actions

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 8, 2022
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 8, 2022
@compiler-errors
Copy link
Member

Same failure as referenced in #95775 (comment) ?

@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 Apr 8, 2022
@bors
Copy link
Contributor

bors commented Apr 8, 2022

⌛ Testing commit b30bcfa with merge 18cc4c9bd0d07bbdcc76a5dbfeb876f6b0e61322...

@bors
Copy link
Contributor

bors commented Apr 8, 2022

💔 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 Apr 8, 2022
@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
Some tests failed in compiletest suite=ui-fulldeps mode=ui host=x86_64-pc-windows-msvc target=x86_64-pc-windows-msvc

error: test run failed!
status: exit code: 101
command: PATH="D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2\lib\rustlib\x86_64-pc-windows-msvc\lib;C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\Program Files (x86)\Windows Kits\10\bin\10.0.22000.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\x64;D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage0-bootstrap-tools\x86_64-pc-windows-msvc\release\deps;D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage0\bin;D:\a\rust\rust\ninja;D:\a\rust\rust\msys2\mingw64\bin;C:\hostedtoolcache\windows\Python\3.10.4\x64\Scripts;C:\hostedtoolcache\windows\Python\3.10.4\x64;C:\msys64\usr\bin;D:\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.7.5\x64;C:\cabal\bin;C:\ghcup\bin;C:\tools\ghc-9.2.2\bin;C:\Program Files\dotnet;C:\mysql\bin;C:\Program Files\R\R-4.1.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:\hostedtoolcache\windows\go\1.15.15\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:\tools\kotlinc\bin;C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.322-6\x64\bin;C:\npm\prefix;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\Docker;C:\Program Files\PowerShell\7;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files\dotnet;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\nodejs;C:\Program Files\OpenSSL\bin;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.5\bin;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;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" "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\test\\ui-fulldeps\\stdio-from\\a.exe"
stdout: none
--- stderr -------------------------------
I/O error: operation failed to complete synchronously
thread 'main' panicked at 'assertion failed: child2.wait()?.success()', D:\a\rust\rust\src/test\ui-fulldeps\stdio-from.rs:50:5
------------------------------------------




failures:
    [ui] src/test\ui-fulldeps\stdio-from.rs

test result: FAILED. 65 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out; finished in 25.47s

Build completed unsuccessfully in 0:35:06
make: *** [Makefile:72: ci-subset-1] Error 1

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 8, 2022

@bors retry #95759

@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 Apr 8, 2022
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

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

@lqd
Copy link
Member

lqd commented Apr 8, 2022

edit: this different network error just took 3 hours to be displayed, it's not a new failure...

@bors
Copy link
Contributor

bors commented Apr 8, 2022

⌛ Testing commit b30bcfa with merge f4a7ce9...

@bors
Copy link
Contributor

bors commented Apr 8, 2022

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing f4a7ce9 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 8, 2022
@bors bors merged commit f4a7ce9 into rust-lang:master Apr 8, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 8, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f4a7ce9): comparison url.

Summary:

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 0 1 0 1 0
mean2 N/A 0.3% N/A -0.4% N/A
max N/A 0.3% N/A -0.4% N/A

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type alias impl trait doesn’t properly enforce outlives relations.
9 participants