Skip to content

Make some MIR ref types closer to the source they are lowered from #150171

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

Open
ShoyuVanilla wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Make some MIR ref types closer to the source they are lowered from #150171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6863181
THIR: lower referenced type of `StaticRef` behind a `&raw` into a raw…
ShoyuVanilla Dec 15, 2025
643ed10
Preserve mutabilities of temps when promoting consts
ShoyuVanilla Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,20 @@ impl<'tcx> ThirBuildCx<'tcx> {
Res::Def(DefKind::Static { .. }, id) => {
// this is &raw for extern static or static mut, and & for other statics
let ty = self.tcx.static_ptr_ty(id, self.typing_env);

// If this is behind a `&raw`, set `ty` as a raw ptr to discern it from normal ref
let ty = if let hir::Node::Expr(&hir::Expr {
kind: hir::ExprKind::AddrOf(hir::BorrowKind::Raw, mutbl, _),
..
}) = self.tcx.parent_hir_node(expr.hir_id)
Comment on lines +1262 to +1265
Copy link
Member

@RalfJung RalfJung Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only find &raw one level up, but will fail to detect &raw static.field1.field2 or similar expressions.

I am not very familiar with this code -- isn't there some existing infrastructure to figure out the "place context" we are in?
As usual I am not sure whom to ping for MIR building things...
Cc @oli-obk @matthewjasper @saethlin

Copy link
Member Author

@ShoyuVanilla ShoyuVanilla Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only find &raw one level up, but will fail to detect &raw static.field1.field2 or similar expressions.

Oh, exactly

Copy link
Member Author

@ShoyuVanilla ShoyuVanilla Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also handle cases like &raw const STATIC[idx] or &raw mut STATIC[start..end]?

Coming back to the original issue: since std::ops::Index::index takes &Self as its receiver, the technically correct form would be unsafe { &*&raw const STATIC }[idx]. That said, unsafe { &*&raw const STATIC[idx] } might already be sufficient to convey the user’s intent—that they are explicitly acknowledging and accepting the potential UB. 🤔

&& let ty::Ref(_, ty, mutbl_) = ty.kind()
&& mutbl == *mutbl_
{
Ty::new_ptr(self.tcx, *ty, mutbl)
} else {
ty
};

let kind = if self.tcx.is_thread_local_static(id) {
ExprKind::ThreadLocalRef(id)
} else {
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_mir_transform/src/promote_consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,10 +760,12 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
}

let num_stmts = self.source[loc.block].statements.len();
let new_temp = self.promoted.local_decls.push(LocalDecl::new(
self.source.local_decls[temp].ty,
self.source.local_decls[temp].source_info.span,
));
let new_local_decl = {
let temp = &self.source.local_decls[temp];
let local_decl = LocalDecl::new(temp.ty, temp.source_info.span);
if temp.mutability == Mutability::Not { local_decl.immutable() } else { local_decl }
};
let new_temp = self.promoted.local_decls.push(new_local_decl);
Copy link
Member Author

@ShoyuVanilla ShoyuVanilla Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn’t discussed in
#149973 (comment),
but I think it would be better to syntactically track promoted consts.

For example, the promoted const in the following code:

static FOO: () = {
    let _ = &&FOO;
};

is currently lowered to:

const FOO::promoted[0]: &&() = {
    let mut _0: &&();
    let mut _1: &();
    let mut _2: &();

    bb0: {
        _2 = const {alloc1: &()};
        _1 = &(*_2);
        _0 = &_1;
        return;
    }
}

I think it would make more sense for _2 to be immutable. This would also make it clearer when detecting &STATIC from:

_2 = const {alloc1: &()};
_1 = &(*_2);

It seems very unlikely that _2 would ever be truly mutable (for example, being reassigned with a different value), but anyway _2 being immutable gives some more assurance.

Copy link
Member

@RalfJung RalfJung Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutability of local variables is pretty much entirely irrelevant in MIR. I don't think it's worth changing this, and it definitely should be separated from the other change which is about the actual types we see in MIR.


debug!("promote({:?} @ {:?}/{:?}, {:?})", temp, loc, num_stmts, self.keep_original);

Expand Down
4 changes: 2 additions & 2 deletions ...opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir
View file
Open in desktop
Copy link
Member

@RalfJung RalfJung Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of these tests actually demonstrate the type of these temporaries changing, right? This just removes some mutability, which is largely irrelevant on the MIR level.

Could you add a test that shows the new logic in action? Ideally add the test in a first commit before the other changes, and then in the 2nd commit just have the diff of what that commit does to the test, so one can see it in action.

Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

const BAR::promoted[0]: &[&i32; 1] = {
let mut _0: &[&i32; 1];
let mut _1: [&i32; 1];
let _1: [&i32; 1];
let mut _2: &i32;
let mut _3: &i32;
let _3: &i32;

bb0: {
_3 = const {ALLOC0: &i32};
Expand Down
4 changes: 2 additions & 2 deletions ...opt/const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-pre-optimizations.after.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

const FOO::promoted[0]: &[&i32; 1] = {
let mut _0: &[&i32; 1];
let mut _1: [&i32; 1];
let _1: [&i32; 1];
let mut _2: &i32;
let mut _3: *const i32;
let _3: *const i32;

bb0: {
_3 = const {ALLOC0: *const i32};
Expand Down
Loading