Support Option and similar enums as type of static variable with linkage attribute #104799

pcc · 2022-11-24T02:24:27Z

Compiler MCP:
rust-lang/compiler-team#565

rustbot · 2022-11-24T02:24:34Z

r? @fee1-dead

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2022-11-24T02:24:37Z

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

rustc_error_messages was changed

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

fee1-dead · 2022-11-26T11:30:46Z

Please fix the CI errors

bjorn3 · 2022-11-29T21:37:29Z

compiler/rustc_codegen_gcc/src/consts.rs

        // Declare a symbol `foo` with the desired linkage.
-        let global1 = cx.declare_global_with_linkage(&sym, llty2, base::global_linkage_to_gcc(linkage));
+        let global1 = cx.declare_global_with_linkage(&sym, cx.type_i8(), base::global_linkage_to_gcc(linkage));


I don't think this is right.

This might need to do something else with GCC, but I don't have a setup for testing the GCC backend.

I asked around on Zulip and I was told that developers do not need to keep the GCC backend working; their changes just needs to pass CI.

compiler/rustc_codegen_llvm/src/consts.rs

compiler/rustc_hir_analysis/src/check/check.rs

compiler/rustc_error_codes/src/error_codes/E0791.md

fee1-dead · 2022-12-02T15:44:49Z

r? @tmiasko

(I am not familiar with this area of the code)

bors · 2022-12-03T18:49:57Z

☔ The latest upstream changes (presumably #97485) made this pull request unmergeable. Please resolve the merge conflicts.

tmiasko · 2022-12-05T13:01:47Z

Thanks, looks good. r=me after rebasing.

This ensures that the error is printed even for unused variables, as well as unifying the handling between the LLVM and GCC backends. This also fixes unusual behavior around exported Rust-defined variables with linkage attributes. With the previous behavior, it appears to be impossible to define such a variable such that it can actually be imported and used by another crate. This is because on the importing side, the variable is required to be a pointer, but on the exporting side, the type checker rejects static variables of pointer type because they do not implement `Sync`. Even if it were possible to import such a type, it appears that code generation on the importing side would add an unexpected additional level of pointer indirection, which would break type safety. This highlighted that the semantics of linkage on Rust-defined variables is different to linkage on foreign items. As such, we now model the difference with two different codegen attributes: linkage for Rust-defined variables, and import_linkage for foreign items. This change gives semantics to the test src/test/ui/linkage-attr/auxiliary/def_illtyped_external.rs which was previously expected to fail to compile. Therefore, convert it into a test that is expected to successfully compile. The update to the GCC backend is speculative and untested.

…age attribute. Compiler MCP: rust-lang/compiler-team#565

tmiasko · 2022-12-06T00:19:35Z

@bors r+

bors · 2022-12-06T00:19:37Z

📌 Commit b4278b0 has been approved by tmiasko

It is now in the queue for this repository.

bors · 2022-12-07T09:09:09Z

⌛ Testing commit b4278b0 with merge 7d28a6aab16bd0c31e10f7aaf7bcfa9aa2f1d8af...

bors · 2022-12-07T09:10:25Z

💔 Test failed - checks-actions

tmiasko · 2022-12-07T09:44:03Z

@bors retry (cancelled without logs)

bors · 2022-12-07T10:25:03Z

⌛ Testing commit b4278b0 with merge 91b8f34...

rust-log-analyzer · 2022-12-07T10:50:54Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

bors · 2022-12-07T13:05:31Z

☀️ Test successful - checks-actions
Approved by: tmiasko
Pushing 91b8f34 to master...

rust-timer · 2022-12-07T14:58:39Z

Finished benchmarking commit (91b8f34): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was 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)	-1.4%	[-2.6%, -0.3%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

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

Cycles

Results

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

…with linkage attribute" cc rust-lang/rust#104799

Support Option and similar enums as type of static variable with linkage attribute Compiler MCP: rust-lang/compiler-team#565

rustbot assigned fee1-dead Nov 24, 2022

pcc force-pushed the linkage-fn branch from ed6c36c to 9071f2b Compare November 24, 2022 02:38

This comment has been minimized.

Sign in to view

pcc force-pushed the linkage-fn branch from 9071f2b to e4ef2c7 Compare November 29, 2022 20:05

This comment has been minimized.

Sign in to view

bjorn3 reviewed Nov 29, 2022

View reviewed changes

compiler/rustc_codegen_llvm/src/consts.rs Show resolved Hide resolved

pcc force-pushed the linkage-fn branch from e4ef2c7 to a0c1e7b Compare December 2, 2022 07:00

tmiasko reviewed Dec 2, 2022

View reviewed changes

rustbot assigned tmiasko and unassigned fee1-dead Dec 2, 2022

pcc force-pushed the linkage-fn branch from a0c1e7b to 1d9cf20 Compare December 3, 2022 01:20

pcc added 3 commits December 5, 2022 15:05

Support Option and similar enums as type of static variable with link…

f44a015

…age attribute. Compiler MCP: rust-lang/compiler-team#565

Reimplement weak! using Option.

b4278b0

pcc force-pushed the linkage-fn branch from 1d9cf20 to b4278b0 Compare December 5, 2022 23:38

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 Dec 6, 2022

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 Dec 7, 2022

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 Dec 7, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 7, 2022

bors merged commit 91b8f34 into rust-lang:master Dec 7, 2022

rustbot added this to the 1.67.0 milestone Dec 7, 2022

bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Dec 11, 2022

Fix for "Support Option and similar enums as type of static variable …

21f4355

…with linkage attribute" cc rust-lang/rust#104799

CAD97 mentioned this pull request Dec 13, 2022

Roadmap for supporting Rust in apps android/ndk#1742

Open

29 tasks

Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023

Auto merge of rust-lang#104799 - pcc:linkage-fn, r=tmiasko

4fcc512

Support Option and similar enums as type of static variable with linkage attribute Compiler MCP: rust-lang/compiler-team#565

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Option and similar enums as type of static variable with linkage attribute #104799

Support Option and similar enums as type of static variable with linkage attribute #104799

pcc commented Nov 24, 2022

rustbot commented Nov 24, 2022

rustbot commented Nov 24, 2022

This comment has been minimized.

fee1-dead commented Nov 26, 2022

This comment has been minimized.

bjorn3 Nov 29, 2022

pcc Nov 29, 2022

fee1-dead commented Dec 2, 2022

bors commented Dec 3, 2022

tmiasko commented Dec 5, 2022

tmiasko commented Dec 6, 2022

bors commented Dec 6, 2022

bors commented Dec 7, 2022

bors commented Dec 7, 2022

tmiasko commented Dec 7, 2022

bors commented Dec 7, 2022

rust-log-analyzer commented Dec 7, 2022

bors commented Dec 7, 2022

rust-timer commented Dec 7, 2022

Support Option and similar enums as type of static variable with linkage attribute #104799

Support Option and similar enums as type of static variable with linkage attribute #104799

Conversation

pcc commented Nov 24, 2022

rustbot commented Nov 24, 2022

rustbot commented Nov 24, 2022

This comment has been minimized.

fee1-dead commented Nov 26, 2022

This comment has been minimized.

bjorn3 Nov 29, 2022

Choose a reason for hiding this comment

pcc Nov 29, 2022

Choose a reason for hiding this comment

fee1-dead commented Dec 2, 2022

bors commented Dec 3, 2022

tmiasko commented Dec 5, 2022

tmiasko commented Dec 6, 2022

bors commented Dec 6, 2022

bors commented Dec 7, 2022

bors commented Dec 7, 2022

tmiasko commented Dec 7, 2022

bors commented Dec 7, 2022

rust-log-analyzer commented Dec 7, 2022

bors commented Dec 7, 2022

rust-timer commented Dec 7, 2022

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles