Skip to content
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

Doc comment custom MIR debuginfo. #117015

Closed
wants to merge 3 commits into from
Closed

Doc comment custom MIR debuginfo. #117015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23052ad
Doc comment custom MIR debuginfo.
cjgillot Oct 21, 2023
775d1c0
Add example.
cjgillot Oct 21, 2023
f5ddec8
FileCheck debuginfo.
cjgillot Oct 21, 2023
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
23 changes: 23 additions & 0 deletions library/core/src/intrinsics/mir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,29 @@
//! - [`Call`] has an associated function as well. The third argument of this function is a normal
//! function call expression, for example `my_other_function(a, 5)`.
//!
//! #### Debuginfo
//!
//! - A debuginfo name can be given to a local using `debug my_name => contents;`.
//! For `contents`, we use the same syntax as operands, to support both places and constants.
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +254 to +255
Copy link
Member

@RalfJung RalfJung Oct 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
//! - A debuginfo name can be given to a local using `debug my_name => contents;`.
//! For `contents`, we use the same syntax as operands, to support both places and constants.
Debuginfo associates source code variable names (of variables that
may not exist any more) with MIR expressions that indicate where
the value of that variable is stored. The syntax to do so is:
```
debug source_var_name => expression;
```
For `expression`, we use the same syntax as operands, to support both places and constants.

//!
//! ```rust
//! #![allow(internal_features)]
//! #![feature(core_intrinsics, custom_mir)]
//!
//! use core::intrinsics::mir::*;
//!
//! #[custom_mir(dialect = "built")]
//! fn debuginfo(option: Option<&i32>) {
//! mir!(
//! debug option => option;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
//! debug option => option;
//! debug plain_local => option;

?

//! debug projection => *Field::<&i32>(Variant(option, 1), 0);
Copy link
Member

Choose a reason for hiding this comment

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

So what does this mean? There's no local "projection" so I'm confused by both the left-hand side and the right-hand side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This mean debuginfo name projection corresponds to the place (*((_1 as Some).0: &i32)).

Copy link
Member

@RalfJung RalfJung Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

And what does that mean?^^ What's a "debuginfo name"?

If the intention is that this is only understandable for people that know our debuginfo system internals, then I guess it's fine. But as someone who has no idea at all about debuginfo, your explanation tells me nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know the better terminology.
I by "debuginfo name", I mean the string you give to gdb when you want to print the variable.
MIR roughly represents debuginfo as a list of structs { Symbol, SourceScope, Place or Const }, stating "this name at at this point in code corresponds to this memory place".

Copy link
Member

Choose a reason for hiding this comment

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

Ah so it's meant to be the name of the variable in the original source code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.
Should we call it "Source code variable name"?

Copy link
Member

Choose a reason for hiding this comment

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

Yes I like that.

//! debug constant => 5_usize;
//! {
//! Return()
//! }
//! )
//! }
//! ```

#![unstable(
feature = "custom_mir",
Expand Down
10 changes: 10 additions & 0 deletions tests/mir-opt/building/custom/debuginfo.constant.built.after.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// MIR for `constant` after built

fn constant() -> () {
debug scalar => const 5_usize;
let mut _0: ();

bb0: {
return;
}
}
2 changes: 1 addition & 1 deletion tests/mir-opt/building/custom/debuginfo.numbered.built.after.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

fn numbered(_1: (u32, i32)) -> () {
debug first => (_1.0: u32);
debug second => (_1.0: u32);
debug second => (_1.1: i32);
let mut _0: ();

bb0: {
Expand Down
29 changes: 27 additions & 2 deletions tests/mir-opt/building/custom/debuginfo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// skip-filecheck
#![feature(custom_mir, core_intrinsics)]

extern crate core;
Expand All @@ -7,6 +6,8 @@ use core::intrinsics::mir::*;
// EMIT_MIR debuginfo.pointee.built.after.mir
#[custom_mir(dialect = "built")]
fn pointee(opt: &mut Option<i32>) {
// CHECK-LABEL: fn pointee(
// CHECK: debug foo => (((*_1) as variant#1).0: i32);
mir!(
debug foo => Field::<i32>(Variant(*opt, 1), 0);
{
Expand All @@ -18,9 +19,12 @@ fn pointee(opt: &mut Option<i32>) {
// EMIT_MIR debuginfo.numbered.built.after.mir
#[custom_mir(dialect = "analysis", phase = "post-cleanup")]
fn numbered(i: (u32, i32)) {
// CHECK-LABEL: fn numbered(
// CHECK: debug first => (_1.0: u32);
// CHECK: debug second => (_1.1: i32);
mir!(
debug first => i.0;
debug second => i.0;
debug second => i.1;
{
Return()
}
Expand All @@ -32,6 +36,8 @@ struct S { x: f32 }
// EMIT_MIR debuginfo.structured.built.after.mir
#[custom_mir(dialect = "analysis", phase = "post-cleanup")]
fn structured(i: S) {
// CHECK-LABEL: fn structured(
// CHECK: debug x => (_1.0: f32);
mir!(
debug x => i.x;
{
Expand All @@ -43,6 +49,8 @@ fn structured(i: S) {
// EMIT_MIR debuginfo.variant.built.after.mir
#[custom_mir(dialect = "built")]
fn variant(opt: Option<i32>) {
// CHECK-LABEL: fn variant(
// CHECK: debug inner => ((_1 as variant#1).0: i32);
mir!(
debug inner => Field::<i32>(Variant(opt, 1), 0);
{
Expand All @@ -54,6 +62,9 @@ fn variant(opt: Option<i32>) {
// EMIT_MIR debuginfo.variant_deref.built.after.mir
#[custom_mir(dialect = "built")]
fn variant_deref(opt: Option<&i32>) {
// CHECK-LABEL: fn variant_deref(
// CHECK: debug pointer => ((_1 as variant#1).0: &i32);
// CHECK: debug deref => (*((_1 as variant#1).0: &i32));
mir!(
debug pointer => Field::<&i32>(Variant(opt, 1), 0);
debug deref => *Field::<&i32>(Variant(opt, 1), 0);
Expand All @@ -63,10 +74,24 @@ fn variant_deref(opt: Option<&i32>) {
)
}

// EMIT_MIR debuginfo.constant.built.after.mir
#[custom_mir(dialect = "built")]
fn constant() {
// CHECK-LABEL: fn constant(
// CHECK: debug scalar => const 5_usize;
mir!(
debug scalar => 5_usize;
{
Return()
}
)
}

fn main() {
numbered((5, 6));
structured(S { x: 5. });
variant(Some(5));
variant_deref(Some(&5));
pointee(&mut Some(5));
constant();
}