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.

Sign up for GitHub

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

Jump to bottom

-Ztranslate-additional-ftl for a broken fluent bundle with stray interpolation argument unexpectedly ICEs #135817

Open
jieyouxu opened this issue Jan 21, 2025 · 2 comments
Open

-Ztranslate-additional-ftl for a broken fluent bundle with stray interpolation argument unexpectedly ICEs #135817

jieyouxu opened this issue Jan 21, 2025 · 2 comments
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

https://github.com/rust-lang/rust/blob/ebbe63891f1fae21734cb97f2f863b08b1d44bf8/tests/run-make/translation/Makefile has a test comment and test case:

rust/tests/run-make/translation/Makefile

Lines 23 to 26 in ebbe638

# Check that a primary bundle with a broken message (e.g. a interpolated
# variable is missing) will use the fallback bundle.
missing: test.rs missing.ftl
$(RUSTC) $< -Ztranslate-additional-ftl=$(CURDIR)/missing.ftl 2>&1 | $(CGREP) "struct literal body without path"

Ignoring that this is using the wrong .ftl file (it should be using broken.ftl to correspond to the test comment, where broken.ftl contains a slug that has a stray fluent interpolation argument), this must have regressed over the years because

rustc test.rs -Ztranslate-additional-ftl=broken.ftl

actually ICEs locally due to

thread 'rustc' panicked at compiler/rustc_errors/src/emitter.rs:1465:84:
called `Result::unwrap()` on an `Err` value: failed while formatting fluent string `parse_struct_literal_body_without_path`:
the fluent string has an argument `foo` that was not found.
help: no arguments are available

Pinging back to #132181.

I assume this is not the intended behavor.
@jieyouxu jieyouxu added A-run-make Area: port run-make Makefiles to rmake.rs A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 21, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 21, 2025
@jieyouxu jieyouxu added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 21, 2025
@jieyouxu jieyouxu changed the title Discrepancy between tests/run-make/translation test comment versus actual behavior -Ztranslate-additional-ftl=broken.ftl for a broken fluent bundle with stray interpolation argument unexpectedly ICEs Jan 21, 2025
@jieyouxu jieyouxu changed the title -Ztranslate-additional-ftl=broken.ftl for a broken fluent bundle with stray interpolation argument unexpectedly ICEs -Ztranslate-additional-ftl for a broken fluent bundle with stray interpolation argument unexpectedly ICEs Jan 21, 2025
@jieyouxu
Copy link
Member Author

jieyouxu commented Jan 21, 2025 •
edited
Loading

The Makefile test didn't catch it because it didn't set

SHELL=/bin/bash -o pipefail

and the Makefile test indeed fails against latest master if pipefail is set due to the ICE.

@jieyouxu jieyouxu mentioned this issue Jan 21, 2025
Merged
jieyouxu added a commit to jieyouxu/rust that referenced this issue Jan 22, 2025
@jieyouxu
Rollup merge of rust-lang#135818 - jieyouxu:migrate-translation, r=co…
315ad11 
…mpiler-errors

tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with `@Oneirical.`

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it ICEs with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 25, 2025
@matthiaskrgr
Rollup merge of rust-lang#135818 - jieyouxu:migrate-translation, r=co…
3cc4dd1 
…mpiler-errors

tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with `@Oneirical.`

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it ICEs with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 25, 2025
@matthiaskrgr
Rollup merge of rust-lang#135818 - jieyouxu:migrate-translation, r=co…
172dcf2 
…mpiler-errors

tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with ``@Oneirical.``

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it ICEs with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 25, 2025
@matthiaskrgr
Rollup merge of rust-lang#135818 - jieyouxu:migrate-translation, r=co…
84b9a71 
…mpiler-errors

tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with ```@Oneirical.```

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it ICEs with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 25, 2025
@bors
Auto merge of rust-lang#135818 - jieyouxu:migrate-translation, r=<try>
8784836 
tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with `@Oneirical.`

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it ICEs with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler

try-job: aarch64-apple
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 25, 2025
@bors
Auto merge of rust-lang#135818 - jieyouxu:migrate-translation, r=<try>
93fdfc4 
tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with `@Oneirical.`

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it ICEs with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler

try-job: aarch64-apple
@jieyouxu
Copy link
Member Author

What's stranger is that this triggers an early ICE on Windows and Linux about the missing fluent interpolation variable, but not aarch64-apple-darwin because reasons?

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 26, 2025
@bors
Auto merge of rust-lang#135818 - jieyouxu:migrate-translation, r=comp…
288b058 
…iler-errors

tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with `@Oneirical.`

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it fails with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler

try-job: aarch64-apple
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 28, 2025
@bors
Auto merge of rust-lang#135818 - jieyouxu:migrate-translation, r=comp…
f0c23fc 
…iler-errors

tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with `@Oneirical.`

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it fails with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler

try-job: aarch64-apple
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 29, 2025
@bors
Auto merge of rust-lang#135818 - jieyouxu:migrate-translation, r=<try>
a459f75 
tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with `@Oneirical.`

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it fails with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler

try-job: aarch64-apple
try-job: i686-mingw
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 29, 2025
@bors
Auto merge of rust-lang#135818 - jieyouxu:migrate-translation, r=comp…
4a5f1cc 
…iler-errors

tests: Port `translation` to rmake.rs

Part of rust-lang#121876.

This PR partially supersedes rust-lang#129011 and is co-authored with `@Oneirical.`

## Summary

This PR ports `tests/run-make/translation` to rmake.rs. Notable changes from the Makefile version include:

- We now actually fail if the rustc invocations fail... The Makefile did not have `SHELL=/bin/bash -o pipefail`, so all the piped rustc invocations to grep vacuously succeeded, even if the broken ftl test case actually regressed over time and ICEs on current master.
    - That test case is converted to assert it fails with a FIXME backlinking to rust-lang#135817.
- The test coverage is expanded to not ignore windows. Instead, the test now uses symlink capability detection to gate test execution.
- Added some backlinks to relevant tracking issues and the initial translation infra implementation PR.

## Review advice

Best reviewed commit-by-commit.

r? compiler

try-job: aarch64-apple
try-job: i686-mingw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jieyouxu @rustbot