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.

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

Conversation

cjgillot
Copy link
Contributor

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 21, 2023
@cjgillot cjgillot force-pushed the custom-mir-debuginfo-doc branch from 52e7328 to 23052ad Compare October 21, 2023 12:03
@cjgillot cjgillot force-pushed the custom-mir-debuginfo-doc branch from 9fa3469 to f5ddec8 Compare October 21, 2023 12:13
//! fn debuginfo(option: Option<&i32>) {
//! mir!(
//! debug option => 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

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.

//! #[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;

?

Comment on lines +254 to +255
//! - 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.
Copy link
Member

@RalfJung RalfJung Oct 22, 2023

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.

@RalfJung
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 29, 2023
@bors
Copy link
Contributor

bors commented Jan 4, 2024

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

@RalfJung
Copy link
Member

@cjgillot what's the status of this PR? I think we have reached a consensus above for how to document this, it just needs to be implemented. :)

@alex-semenyuk alex-semenyuk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 7, 2024
@JohnCSimon
Copy link
Member

Ping from triage: I'm closing this due to inactivity because no commits have occurred in a year. Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Nov 29, 2024
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Nov 29, 2024
@RalfJung
Copy link
Member

I revived this PR at #133625

jieyouxu added a commit to jieyouxu/rust that referenced this pull request Nov 30, 2024
…compiler-errors

custom MIR: add doc comment for debuginfo

This is a revival of rust-lang#117015
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Nov 30, 2024
…compiler-errors

custom MIR: add doc comment for debuginfo

This is a revival of rust-lang#117015
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2024
Rollup merge of rust-lang#133625 - RalfJung:custom-mir-debug-info, r=compiler-errors

custom MIR: add doc comment for debuginfo

This is a revival of rust-lang#117015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants