Skip to content

Conversation

Zalathar
Copy link
Contributor

The previous parser for //@ normalize-* headers (before #126370) was so lax that it did not require : after the header name. As a result, the test suite contained a mix of with-colon and without-colon normalize headers, both numbering in the hundreds.

This PR updates the without-colon headers to add a colon (matching the style used by other headers), and then updates the parser to make the colon mandatory.

(Because the normalization parser only runs after the header system identifies a normalize header, this will detect and issue an error for relevant headers that lack the colon.)

Addresses one of the points of #126372.

@rustbot
Copy link
Collaborator

rustbot commented Jun 21, 2024

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 21, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 21, 2024

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@Zalathar
Copy link
Contributor Author

The pattern I used for find/replace was:

  • ([ @\]]normalize-[a-z0-9-]*)
  • $1:

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@lcnr
Copy link
Contributor

lcnr commented Jul 10, 2024

unsure whether we've added any normalize directives in the last few weeks, so let's go with

@bors delegate+

@bors r+ rollup=never

@bors
Copy link
Collaborator

bors commented Jul 10, 2024

✌️ @Zalathar, you can now approve this pull request!

If @lcnr told you to "r=me" after making some further change, please make that change, then do @bors r=@lcnr

@bors
Copy link
Collaborator

bors commented Jul 10, 2024

📌 Commit 27db617 has been approved by lcnr

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 10, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 10, 2024
Require a colon in `//@ normalize-*:` test headers

The previous parser for `//@ normalize-*` headers (before rust-lang#126370) was so lax that it did not require `:` after the header name. As a result, the test suite contained a mix of with-colon and without-colon normalize headers, both numbering in the hundreds.

This PR updates the without-colon headers to add a colon (matching the style used by other headers), and then updates the parser to make the colon mandatory.

(Because the normalization parser only runs *after* the header system identifies a normalize header, this will detect and issue an error for relevant headers that lack the colon.)

Addresses one of the points of rust-lang#126372.
@bors
Copy link
Collaborator

bors commented Jul 10, 2024

⌛ Testing commit 27db617 with merge 806cfe2...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Jul 10, 2024

💔 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 Jul 10, 2024
@Zalathar
Copy link
Contributor Author

Indeed, there have been a few normalize directives without a colon added over the last few weeks (diff).

@Zalathar
Copy link
Contributor Author

PR CI is green; let's try this again.

@bors r=lcnr

@bors
Copy link
Collaborator

bors commented Jul 11, 2024

📌 Commit b677359 has been approved by lcnr

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 11, 2024
@Zalathar
Copy link
Contributor Author

Seeing actual failures makes me think that the error message could use some improvement, but I'd rather get the check merged before it rots again. I can worry about diagnostics in a follow-up.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 11, 2024
Require a colon in `//@ normalize-*:` test headers

The previous parser for `//@ normalize-*` headers (before rust-lang#126370) was so lax that it did not require `:` after the header name. As a result, the test suite contained a mix of with-colon and without-colon normalize headers, both numbering in the hundreds.

This PR updates the without-colon headers to add a colon (matching the style used by other headers), and then updates the parser to make the colon mandatory.

(Because the normalization parser only runs *after* the header system identifies a normalize header, this will detect and issue an error for relevant headers that lack the colon.)

Addresses one of the points of rust-lang#126372.
@bors
Copy link
Collaborator

bors commented Jul 11, 2024

⌛ Testing commit b677359 with merge 6f37281...

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
   Compiling rustc_driver v0.0.0 (C:\a\rust\rust\compiler\rustc_driver)
[RUSTC-TIMING] rustc_driver test:false 4.472
error: linking with `link.exe` failed: exit code: 1104
  |
  = note: "C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Tools\\MSVC\\14.40.33807\\bin\\HostX64\\x64\\link.exe" "/NOLOGO" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\symbols.o" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1-rustc\\x86_64-pc-windows-msvc\\release\\deps\\rustc_main-aa69889b5edbcee3.rustc_main.dc4c9f64bdd047d3-cgu.0.rcgu.o" "/LIBPATH:C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1-rustc\\x86_64-pc-windows-msvc\\release\\deps" "/LIBPATH:C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1-rustc\\release\\deps" "/LIBPATH:C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Tools\\MSVC\\14.40.33807\\atlmfc\\lib\\x64" "/LIBPATH:C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1-rustc\\x86_64-pc-windows-msvc\\release\\build\\stacker-789dd3deafe57b42\\out" "/LIBPATH:C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Tools\\MSVC\\14.40.33807\\atlmfc\\lib\\x64" "/LIBPATH:C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1-rustc\\x86_64-pc-windows-msvc\\release\\build\\psm-7fda0fde8da92b20\\out" "/LIBPATH:C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Tools\\MSVC\\14.40.33807\\atlmfc\\lib\\x64" "/LIBPATH:C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1-rustc\\x86_64-pc-windows-msvc\\release\\build\\rustc_llvm-4d68b412bbca4af1\\out" "/LIBPATH:C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\llvm\\lib" "/LIBPATH:C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1-rustc\\x86_64-pc-windows-msvc\\release\\deps\\rustc_driver-0eb24ce336db85be.dll.lib" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\std-cef76c2685dfb4ca.dll.lib" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libcompiler_builtins-a6954cc40265b962.rlib" "psapi.lib" "shell32.lib" "ole32.lib" "uuid.lib" "advapi32.lib" "ws2_32.lib" "advapi32.lib" "kernel32.lib" "ole32.lib" "oleaut32.lib" "bcrypt.lib" "advapi32.lib" "advapi32.lib" "cfgmgr32.lib" "gdi32.lib" "kernel32.lib" "msimg32.lib" "opengl32.lib" "user32.lib" "winspool.lib" "legacy_stdio_definitions.lib" "kernel32.lib" "advapi32.lib" "ntdll.lib" "userenv.lib" "ws2_32.lib" "kernel32.lib" "kernel32.lib" "/defaultlib:libcmt" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\advapi32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-errorhandling-l1-1-3.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-file-fromapp-l1-1-0.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-handle-l1-1-0.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-ioring-l1-1-0.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-synch-l1-2-0.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-sysinfo-l1-2-0.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-sysinfo-l1-2-3.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-sysinfo-l1-2-4.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-util-l1-1-1.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-core-wow64-l1-1-1.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\api-ms-win-security-base-l1-2-2.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\avrt.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\bcp47mrm.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\bcryptprimitives.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\clfsw32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\dbghelp.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\elscore.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\fwpuclnt.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\gdi32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\icu.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\imagehlp.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\iphlpapi.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\kernel32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\ktmw32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\mswsock.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\netapi32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\normaliz.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\ntdll.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\ntdllk.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\ole32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\oleaut32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\psapi.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\rtworkq.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\txfw32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\user32.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\usp10.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\version.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\windows.networking.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\wofutil.dll_imports_indirect.lib" "C:\\a\\_temp\\msys64\\tmp\\rustcFuM9tk\\ws2_32.dll_imports_indirect.lib" "/NXCOMPAT" "/LIBPATH:C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "/OUT:C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1-rustc\\x86_64-pc-windows-msvc\\release\\deps\\rustc_main-aa69889b5edbcee3.exe" "/OPT:REF,ICF" "/DEBUG" "/PDBALTPATH:%_PDB%" "/MANIFEST:EMBED" "/MANIFESTINPUT:C:\\a\\rust\\rust\\compiler\\rustc\\Windows Manifest.xml" "/WX"
  = note: LINK : fatal error LNK1104: cannot open file 'C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-rustc\x86_64-pc-windows-msvc\release\deps\rustc_main-aa69889b5edbcee3.exe'
          

[RUSTC-TIMING] rustc_main test:false 0.578
error: could not compile `rustc-main` (bin "rustc-main") due to 1 previous error

@bors
Copy link
Collaborator

bors commented Jul 11, 2024

💔 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 Jul 11, 2024
@lcnr
Copy link
Contributor

lcnr commented Jul 11, 2024

linker error

@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 Jul 11, 2024
@bors
Copy link
Collaborator

bors commented Jul 11, 2024

⌛ Testing commit b677359 with merge fdf7ea6...

@bors
Copy link
Collaborator

bors commented Jul 11, 2024

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing fdf7ea6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 11, 2024
@bors bors merged commit fdf7ea6 into rust-lang:master Jul 11, 2024
@rustbot rustbot added this to the 1.81.0 milestone Jul 11, 2024
@Zalathar Zalathar deleted the normalize-colon branch July 11, 2024 11:50
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fdf7ea6): 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 (primary 2.1%)

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)
2.1% [2.1%, 2.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.1% [2.1%, 2.1%] 1

Cycles

Results (secondary -4.3%)

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)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.3% [-5.9%, -3.3%] 6
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 703.344s -> 703.866s (0.07%)
Artifact size: 328.69 MiB -> 328.57 MiB (-0.03%)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 12, 2024
…iser

compiletest: Better error message for bad `normalize-*` headers

Follow-up to rust-lang#126777.

Example of the new error message in context:
```text
---- [ui] tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs stdout ----
thread '[ui] tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs' panicked at src/tools/compiletest/src/header.rs:1001:13:
couldn't parse custom normalization rule: `normalize-stderr-test ".*note: .*\n\n" -> ""`
help: expected syntax is: `normalize-stderr-test: "REGEX" -> "REPLACEMENT"`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 12, 2024
Rollup merge of rust-lang#127607 - Zalathar:normalize-hint, r=wesleywiser

compiletest: Better error message for bad `normalize-*` headers

Follow-up to rust-lang#126777.

Example of the new error message in context:
```text
---- [ui] tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs stdout ----
thread '[ui] tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs' panicked at src/tools/compiletest/src/header.rs:1001:13:
couldn't parse custom normalization rule: `normalize-stderr-test ".*note: .*\n\n" -> ""`
help: expected syntax is: `normalize-stderr-test: "REGEX" -> "REPLACEMENT"`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-testsuite Area: The testsuite used to check the correctness of rustc 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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

6 participants