Throw error on failure in loading llvm-plugin

[dfordivam]

The following code silently ignores the error as the LLVMRustSetLastError only tracks one error at a time. At all other places where LLVMRustSetLastError is used the code immediately returns.

rust/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

Lines 801 to 804 in 251831e

	if (!Plugin) {
	LLVMRustSetLastError(("Failed to load pass plugin" + PluginPath.str()).c_str());
	continue;
	}

[rustbot]

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

Please see the contribution instructions for more information.

[cuviper]

cc #91125 @eskarn @nagisa

I think this is intentional to continue without the plugin, but you're right that LLVMRustSetLastError isn't useful if we don't actually indicate a failure. The similar code in opt prints that it's ignored and returns from its callback:
https://github.com/llvm/llvm-project/blob/96c037ef9c01e5e26c84e571feb3e5c16519727d/llvm/tools/opt/opt.cpp#L503-L508

[nagisa]

A non-functional-change would be to concatenate all the plugins that failed into the error message and return it after the loop is done, though in practice I don't think it is a huge deal either way.

[dfordivam]

In the legacy pass manager, not able to load a plugin used to be an error / panic. So I am not sure why the behavior should change with the new pass manager.

rust/compiler/rustc_codegen_llvm/src/llvm_util.rs

Line 130 in a37499a

let lib = Library::new(plugin).unwrap_or_else(|e| bug!("couldn't load plugin: {}", e));

Concatenating the errors is definitely better, but as mentioned it isn't a big deal either way.

[cuviper]

In the legacy pass manager, not able to load a plugin used to be an error / panic. So I am not sure why the behavior should change with the new pass manager.

OK, fair enough!

@bors r+

[bors]

📌 Commit 693c631 has been approved by cuviper

It is now in the queue for this repository.