Auto merge of rust-lang#71916 - Dylan-DPC:rollup-luj7zx3, r=Dylan-DPC

Rollup of 4 pull requests

Successful merges:

 - rust-lang#69984 (Add Option to Force Unwind Tables)
 - rust-lang#71830 (Remove clippy from some leftover lists of "possibly failing" tools)
 - rust-lang#71894 (Suggest removing semicolon in last expression only if it's type is known)
 - rust-lang#71897 (Improve docs for embed-bitcode and linker-plugin-lto)

Failed merges:

r? @ghost

Author: bors

src/bootstrap/test.rs

@@ -528,7 +528,7 @@ impl Step for Clippy {
                 host,
                 "test",
                 "src/tools/clippy",
-                SourceType::Submodule,
+                SourceType::InTree,
                 &[],
             );
 

src/bootstrap/toolstate.rs

@@ -78,7 +78,6 @@ static STABLE_TOOLS: &[(&str, &str)] = &[
     ("edition-guide", "src/doc/edition-guide"),
     ("rls", "src/tools/rls"),
     ("rustfmt", "src/tools/rustfmt"),
-    ("clippy-driver", "src/tools/clippy"),
 ];
 
 // These tools are permitted to not build on the beta/stable channels.

src/doc/rustc/src/codegen-options/index.md

@@ -62,24 +62,40 @@ the linker.
 
 ## embed-bitcode
 
-This flag controls whether or not the compiler puts LLVM bitcode into generated
-rlibs. It takes one of the following values:
+This flag controls whether or not the compiler embeds LLVM bitcode into object
+files. It takes one of the following values:
 
 * `y`, `yes`, `on`, or no value: put bitcode in rlibs (the default).
 * `n`, `no`, or `off`: omit bitcode from rlibs.
 
-LLVM bitcode is only needed when link-time optimization (LTO) is being
-performed, but it is enabled by default for backwards compatibility reasons.
+LLVM bitcode is required when rustc is performing link-time optimization (LTO).
+It is also required on some targets like iOS ones where vendors look for LLVM
+bitcode. Embedded bitcode will appear in rustc-generated object files inside of
+a section whose name is defined by the target platform. Most of the time this is
+`.llvmbc`.
 
 The use of `-C embed-bitcode=no` can significantly improve compile times and
-reduce generated file sizes. For these reasons, Cargo uses `-C
-embed-bitcode=no` whenever possible. Likewise, if you are building directly
-with `rustc` we recommend using `-C embed-bitcode=no` whenever you are not
-using LTO.
+reduce generated file sizes if your compilation does not actually need bitcode
+(e.g. if you're not compiling for iOS or you're not performing LTO). For these
+reasons, Cargo uses `-C embed-bitcode=no` whenever possible. Likewise, if you
+are building directly with `rustc` we recommend using `-C embed-bitcode=no`
+whenever you are not using LTO.
 
 If combined with `-C lto`, `-C embed-bitcode=no` will cause `rustc` to abort
 at start-up, because the combination is invalid.
 
+> **Note**: if you're building Rust code with LTO then you probably don't even
+> need the `embed-bitcode` option turned on. You'll likely want to use
+> `-Clinker-plugin-lto` instead which skips generating object files entirely and
+> simply replaces object files with LLVM bitcode. The only purpose for
+> `-Cembed-bitcode` is when you're generating an rlib that is both being used
+> with and without LTO. For example Rust's standard library ships with embedded
+> bitcode since users link to it both with and without LTO.
+>
+> This also may make you wonder why the default is `yes` for this option. The
+> reason for that is that it's how it was for rustc 1.44 and prior. In 1.45 this
+> option was added to turn off what had always been the default.
+
 ## extra-filename
 
 This option allows you to put extra data in each output filename. It takes a
@@ -98,6 +114,18 @@ values:
 The default behaviour, if frame pointers are not force-enabled, depends on the
 target.
 
+## force-unwind-tables
+
+This flag forces the generation of unwind tables. It takes one of the following
+values:
+
+* `y`, `yes`, `on`, or no value: Unwind tables are forced to be generated.
+* `n`, `no`, or `off`: Unwind tables are not forced to be generated. If unwind
+  tables are required by the target or `-C panic=unwind`, an error will be
+  emitted.
+
+The default if not specified depends on the target.
+
 ## incremental
 
 This flag allows you to enable incremental compilation, which allows `rustc`
@@ -187,6 +215,18 @@ the following values:
 * `n`, `no`, or `off`: disable linker plugin LTO (the default).
 * A path to the linker plugin.
 
+More specifically this flag will cause the compiler to replace its typical
+object file output with LLVM bitcode files. For example an rlib produced with
+`-Clinker-plugin-lto` will still have `*.o` files in it, but they'll all be LLVM
+bitcode instead of actual machine code. It is expected that the native platform
+linker is capable of loading these LLVM bitcode files and generating code at
+link time (typically after performing optimizations).
+
+Note that rustc can also read its own object files produced with
+`-Clinker-plugin-lto`. If an rlib is only ever going to get used later with a
+`-Clto` compilation then you can pass `-Clinker-plugin-lto` to speed up
+compilation and avoid generating object files that aren't used.
+
 ## llvm-args
 
 This flag can be used to pass a list of arguments directly to LLVM.

src/librustc_codegen_llvm/allocator.rs

@@ -54,7 +54,7 @@ pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut ModuleLlvm, kind: Alloc
         if tcx.sess.target.target.options.default_hidden_visibility {
             llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden);
         }
-        if tcx.sess.target.target.options.requires_uwtable {
+        if tcx.sess.must_emit_unwind_tables() {
             attributes::emit_uwtable(llfn, true);
         }
 

src/librustc_codegen_llvm/attributes.rs

@@ -13,7 +13,6 @@ use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_session::config::{OptLevel, Sanitizer};
 use rustc_session::Session;
-use rustc_target::spec::PanicStrategy;
 
 use crate::attributes;
 use crate::llvm::AttributePlace::Function;
@@ -271,9 +270,7 @@ pub fn from_fn_attrs(cx: &CodegenCx<'ll, 'tcx>, llfn: &'ll Value, instance: ty::
     //
     // You can also find more info on why Windows is whitelisted here in:
     //      https://bugzilla.mozilla.org/show_bug.cgi?id=1302078
-    if cx.sess().panic_strategy() == PanicStrategy::Unwind
-        || cx.sess().target.target.options.requires_uwtable
-    {
+    if cx.sess().must_emit_unwind_tables() {
         attributes::emit_uwtable(llfn, true);
     }
 

src/librustc_interface/tests.rs

@@ -415,6 +415,7 @@ fn test_codegen_options_tracking_hash() {
     tracked!(debuginfo, 0xdeadbeef);
     tracked!(embed_bitcode, false);
     tracked!(force_frame_pointers, Some(false));
+    tracked!(force_unwind_tables, Some(true));
     tracked!(inline_threshold, Some(0xf007ba11));
     tracked!(linker_plugin_lto, LinkerPluginLto::LinkerPluginAuto);
     tracked!(llvm_args, vec![String::from("1"), String::from("2")]);

src/librustc_session/options.rs

@@ -668,6 +668,8 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options,
         "extra data to put in each output filename"),
     force_frame_pointers: Option<bool> = (None, parse_opt_bool, [TRACKED],
         "force use of the frame pointers"),
+    force_unwind_tables: Option<bool> = (None, parse_opt_bool, [TRACKED],
+        "force use of unwind tables"),
     incremental: Option<String> = (None, parse_opt_string, [UNTRACKED],
         "enable incremental compilation"),
     inline_threshold: Option<usize> = (None, parse_opt_uint, [TRACKED],

src/librustc_session/session.rs

@@ -646,6 +646,33 @@ impl Session {
         }
     }
 
+    pub fn must_emit_unwind_tables(&self) -> bool {
+        // This is used to control the emission of the `uwtable` attribute on
+        // LLVM functions.
+        //
+        // At the very least, unwind tables are needed when compiling with
+        // `-C panic=unwind`.
+        //
+        // On some targets (including windows), however, exceptions include
+        // other events such as illegal instructions, segfaults, etc. This means
+        // that on Windows we end up still needing unwind tables even if the `-C
+        // panic=abort` flag is passed.
+        //
+        // You can also find more info on why Windows needs unwind tables in:
+        //      https://bugzilla.mozilla.org/show_bug.cgi?id=1302078
+        //
+        // If a target requires unwind tables, then they must be emitted.
+        // Otherwise, we can defer to the `-C force-unwind-tables=<yes/no>`
+        // value, if it is provided, or disable them, if not.
+        if self.panic_strategy() == PanicStrategy::Unwind {
+            true
+        } else if self.target.target.options.requires_uwtable {
+            true
+        } else {
+            self.opts.cg.force_unwind_tables.unwrap_or(false)
+        }
+    }
+
     /// Returns the symbol name for the registrar function,
     /// given the crate `Svh` and the function `DefIndex`.
     pub fn generate_plugin_registrar_symbol(&self, disambiguator: CrateDisambiguator) -> String {
@@ -1224,6 +1251,23 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
         }
     }
 
+    // Unwind tables cannot be disabled if the target requires them.
+    if let Some(include_uwtables) = sess.opts.cg.force_unwind_tables {
+        if sess.panic_strategy() == PanicStrategy::Unwind && !include_uwtables {
+            sess.err(
+                "panic=unwind requires unwind tables, they cannot be disabled \
+                     with `-C force-unwind-tables=no`.",
+            );
+        }
+
+        if sess.target.target.options.requires_uwtable && !include_uwtables {
+            sess.err(
+                "target requires unwind tables, they cannot be disabled with \
+                     `-C force-unwind-tables=no`.",
+            );
+        }
+    }
+
     // PGO does not work reliably with panic=unwind on Windows. Let's make it
     // an error to combine the two for now. It always runs into an assertions
     // if LLVM is built with assertions, but without assertions it sometimes

src/librustc_typeck/check/mod.rs

@@ -5387,7 +5387,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             _ => return None,
         };
         let last_expr_ty = self.node_ty(last_expr.hir_id);
-        if self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err() {
+        if matches!(last_expr_ty.kind, ty::Error)
+            || self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err()
+        {
             return None;
         }
         let original_span = original_sp(last_stmt.span, blk.span);

src/test/codegen/force-unwind-tables.rs

@@ -0,0 +1,7 @@
+// min-llvm-version 8.0
+// compile-flags: -C no-prepopulate-passes -C force-unwind-tables=y
+
+#![crate_type="lib"]
+
+// CHECK: attributes #{{.*}} uwtable
+pub fn foo() {}