Restore behavior when primary bundle is missing

[mejrs]

Fixes #106755 by restoring some of the behavior prior to #106427

Still, I have no idea how this debug assertion can even hit while using en-US as primary bundle.

r? @davidtwco

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @davidtwco (or someone else) soon.

Please see the contribution instructions for more information.

[rustbot]

rustc_errors::translation was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

[davidtwco]

Still, I have no idea how this debug assertion can even hit while using en-US as primary bundle.

If I remember correctly, we lazily load the primary bundle contents, maybe there's a code path that triggers a translation without triggering the lazy load?

[mejrs]

Still, I have no idea how this debug assertion can even hit while using en-US as primary bundle.

If I remember correctly, we lazily load the primary bundle contents, maybe there's a code path that triggers a translation without triggering the lazy load?

If there is one I can't find it, but I do not know this code well.

[davidtwco]

I think we probably want the patch that you have now, so that we can have partially translated primary bundles and not ICE repeatedly when using a debug build.

However, the reason that this fails is that when the default locale is requested, then we ended up just creating an empty bundle with no translated messages, and the patch below fixes that, so when the default locale is requested (and no testing Fluent resource is provided), then we just don't create a primary bundle at all.

diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs
index 1882d4b698e..32aa90afa9b 100644
--- a/compiler/rustc_error_messages/src/lib.rs
+++ b/compiler/rustc_error_messages/src/lib.rs
@@ -176,6 +176,10 @@ pub fn fluent_bundle(

     let fallback_locale = langid!("en-US");
     let requested_fallback_locale = requested_locale.as_ref() == Some(&fallback_locale);
+    trace!(?requested_fallback_locale);
+    if requested_fallback_locale && additional_ftl_path.is_none() {
+        return Ok(None);
+    }

     // If there is only `-Z additional-ftl-path`, assume locale is "en-US", otherwise use user
     // provided locale.
@@ -194,7 +198,7 @@ pub fn fluent_bundle(
     bundle.set_use_isolating(with_directionality_markers);

     // If the user requests the default locale then don't try to load anything.
-    if !requested_fallback_locale && let Some(requested_locale) = requested_locale {
+    if let Some(requested_locale) = requested_locale {
         let mut found_resources = false;
         for sysroot in user_provided_sysroot.iter_mut().chain(sysroot_candidates.iter_mut()) {
             sysroot.push("share");

[davidtwco]

Oh also, I'd appreciate if you added a test for this.

[mejrs]

Thanks, that fixes the problem. However I am not sure how to add a test for it, because it requires a rustc built with debug assertions.

[davidtwco]

Thanks, that fixes the problem. However I am not sure how to add a test for it, because it requires a rustc built with debug assertions.

Just a test with -Ztranslate-lang=en-US that shows the correct output should be enough - some of the builders have debug assertions enabled and the behaviour should now be the same with or without those assertions.

[rustbot]

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @TaKO8Ki

[mejrs]

Also @davidtwco I rebased

<<<<<<< HEAD
                // Always yeet out for errors on debug (unless
                // `RUSTC_TRANSLATION_NO_DEBUG_ASSERT` is set in the environment - this allows
                // local runs of the test suites, of builds with debug assertions, to test the
                // behaviour in a normal build).
                Some(Err(primary))
                    if cfg!(debug_assertions)
                        && env::var("RUSTC_TRANSLATION_NO_DEBUG_ASSERT").is_err() =>
                {
                    do yeet primary
                }
=======
                // If `translate_with_bundle` returns `Err` with the primary bundle, this is likely
                // just that the primary bundle doesn't contain the message being translated, so
                // proceed to the fallback bundle.
                Some(Err(
                    primary @ TranslateError::One {
                        kind: TranslateErrorKind::MessageMissing, ..
                    },
                )) => translate_with_bundle(self.fallback_fluent_bundle())
                    .map_err(|fallback| primary.and(fallback))?,

                // However, when errors are produced from translation, then that means the translation
                // is broken (e.g. `{$foo}` exists in a translation but `foo` isn't provided).
                //
                // In debug builds, assert so that compiler devs can spot the broken translation and
                // fix it..
                Some(Err(primary)) if cfg!(debug_assertions) => do yeet primary,
>>>>>>> 6b812c48ce6 (Restore behavior when primary bundle is missing)

as

                // If `translate_with_bundle` returns `Err` with the primary bundle, this is likely
                // just that the primary bundle doesn't contain the message being translated, so
                // proceed to the fallback bundle.
                Some(Err(
                    primary @ TranslateError::One {
                        kind: TranslateErrorKind::MessageMissing, ..
                    },
                )) => translate_with_bundle(self.fallback_fluent_bundle())
                    .map_err(|fallback| primary.and(fallback))?,

                // Always yeet out for errors on debug (unless
                // `RUSTC_TRANSLATION_NO_DEBUG_ASSERT` is set in the environment - this allows
                // local runs of the test suites, of builds with debug assertions, to test the
                // behaviour in a normal build).
                Some(Err(primary))
                    if cfg!(debug_assertions)
                        && env::var("RUSTC_TRANSLATION_NO_DEBUG_ASSERT").is_err() =>
                {
                    do yeet primary
                }

I think that is right.

[davidtwco]

@bors r+ rollup

[bors]

📌 Commit 4c13a21 has been approved by davidtwco

It is now in the queue for this repository.