Skip to content

fallback default to None during ast-lowering for lifetime binder #119042

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Dec 26, 2023

Conversation

bvanjoi
Copy link
Contributor

@bvanjoi bvanjoi commented Dec 17, 2023

Fixes #118697

This is another attempt. It has a fallback, setting default to None and emit an error for non-lifetime binders during ast lowering.

r? @compiler-errors

@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. labels Dec 17, 2023
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.

Please move this to ast validation or some post-macro-expansion pass.

@compiler-errors 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 Dec 17, 2023
@bvanjoi
Copy link
Contributor Author

bvanjoi commented Dec 18, 2023

I'm pretty sure that this PR will cause the following code to go from pass to fail: ....

Yes, I agree with this, and it's a great catch.

move this to ast validation

But I'm curious about how we can change the default in GenericParamKind::Type from Some(xxx) to None because the validation is simply a visit and can't modify the ast.

@bvanjoi
Copy link
Contributor Author

bvanjoi commented Dec 21, 2023

I have shifted this process into the AST lowering stage: if the default value is not empty, it will raise an error and fall back to None.

@bvanjoi
Copy link
Contributor Author

bvanjoi commented Dec 21, 2023

ci is green. @rustbot ready

@rustbot rustbot 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 Dec 21, 2023
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.

r=me after leaving a comment

@bvanjoi bvanjoi changed the title fallback default to None during parse late-bound lifetime fallback default to None during ast-loweing for lifetime binder Dec 23, 2023
@rustbot

This comment has been minimized.

@bvanjoi bvanjoi changed the title fallback default to None during ast-loweing for lifetime binder fallback default to None during ast-loweing for lifetime binder Dec 23, 2023
@rustbot

This comment has been minimized.

@bvanjoi bvanjoi changed the title fallback default to None during ast-loweing for lifetime binder fallback default to None during ast-loweing for lifetime binder Dec 23, 2023
@rustbot

This comment has been minimized.

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 25, 2023

📌 Commit 873ed19 has been approved by compiler-errors

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 Dec 25, 2023
@fmease fmease changed the title fallback default to None during ast-loweing for lifetime binder fallback default to None during ast-lowering for lifetime binder Dec 26, 2023
@bvanjoi
Copy link
Contributor Author

bvanjoi commented Dec 26, 2023

@rustbot ready

@rustbot rustbot 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 Dec 26, 2023
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 26, 2023

📌 Commit e16efbd has been approved by compiler-errors

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 Dec 26, 2023
@bors
Copy link
Collaborator

bors commented Dec 26, 2023

⌛ Testing commit e16efbd with merge fb3598d...

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 26, 2023
fallback `default` to `None` during ast-lowering for lifetime binder

Fixes rust-lang#118697

This is another attempt. It has a fallback, setting `default` to `None` and emit an error for non-lifetime binders during ast lowering.

r? `@compiler-errors`
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-mingw 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\inline_cross\sugar-closure-crate-21801.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.12.1\x64\Scripts;C:\hostedtoolcache\windows\Python\3.12.1\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.13.1\x64;C:\cabal\bin;C:\ghcup\bin;C:\mingw64\bin;C:\Program Files\dotnet;C:\Program Files\MySQL\MySQL Server 5.7\bin;C:\Program Files\R\R-4.3.2\bin\x64;C:\SeleniumWebDrivers\GeckoDriver;C:\SeleniumWebDrivers\EdgeDriver;C:\SeleniumWebDrivers\ChromeDriver;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.12\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.392-8\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:\ProgramData\Chocolatey\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;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:\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\\inline_cross\\sugar-closure-crate-21801\\auxiliary" "-o" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\rustdoc\\inline_cross\\sugar-closure-crate-21801" "--deny" "warnings" "C:\\a\\rust\\rust\\tests\\rustdoc\\inline_cross\\sugar-closure-crate-21801.rs" "-A" "internal_features"
--- stderr -------------------------------

thread 'main' has overflowed its stack
------------------------------------------

@bors
Copy link
Collaborator

bors commented Dec 26, 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 Dec 26, 2023
@bvanjoi
Copy link
Contributor Author

bvanjoi commented Dec 26, 2023

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

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

This appears to be unrelated to the current change...

@fmease
Copy link
Member

fmease commented Dec 26, 2023

@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 Dec 26, 2023
@bors
Copy link
Collaborator

bors commented Dec 26, 2023

⌛ Testing commit e16efbd with merge a75fed7...

@bors
Copy link
Collaborator

bors commented Dec 26, 2023

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 26, 2023
@bors bors merged commit a75fed7 into rust-lang:master Dec 26, 2023
@rustbot rustbot added this to the 1.77.0 milestone Dec 26, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a75fed7): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -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)
2.3% [2.2%, 2.4%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

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: 670.607s -> 671.414s (0.12%)
Artifact size: 312.44 MiB -> 312.42 MiB (-0.01%)

fmease added a commit to fmease/rust that referenced this pull request Jan 2, 2024
…ompiler-errors

Deny defaults for higher-ranked generic parameters

Fixes rust-lang#119489 (incl. rust-lang#119489 (comment)).
Partially reverts rust-lang#119042.

cc `@bvanjoi`
r? `@compiler-errors` or compiler
fmease added a commit to fmease/rust that referenced this pull request Jan 3, 2024
…ompiler-errors

Deny defaults for higher-ranked generic parameters

Fixes rust-lang#119489 (incl. rust-lang#119489 (comment)).
Partially reverts rust-lang#119042.

cc ``@bvanjoi``
r? ``@compiler-errors`` or compiler
fmease added a commit to fmease/rust that referenced this pull request Jan 3, 2024
…ompiler-errors

Deny defaults for higher-ranked generic parameters

Fixes rust-lang#119489 (incl. rust-lang#119489 (comment)).
Partially reverts rust-lang#119042.

cc ```@bvanjoi```
r? ```@compiler-errors``` or compiler
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 3, 2024
Rollup merge of rust-lang#119494 - fmease:deny-hr-param-defaults, r=compiler-errors

Deny defaults for higher-ranked generic parameters

Fixes rust-lang#119489 (incl. rust-lang#119489 (comment)).
Partially reverts rust-lang#119042.

cc ```@bvanjoi```
r? ```@compiler-errors``` or compiler
# for free to join this conversation on GitHub. Already have an account? # 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.

ICE: ast lowering: no entry for node id: NodeId(26)
8 participants