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

[feat] Optional Inlay Hint for Too Many Implicit Returns in One Function #18924

Open
Tunixer opened this issue Jan 12, 2025 · 9 comments
Open
Labels
A-inlay-hints inlay/inline hints C-feature Category: feature request

Comments

@Tunixer
Copy link

Tunixer commented Jan 12, 2025

I have previewed the history of issues, and noticed that the optional inlay hint for the implicit returns would be useful when there are too many returns in one function.
For example, in crates\hir-def\src\body\lower\asm.rs, the lower_inline_asm function has too many returns for readers to distinguish the difference.
I also see the similar issues in the history, e.g. #9969, #4691.
I think adding inlay hint for the implicit return would be a useful option.

@Tunixer Tunixer added the C-feature Category: feature request label Jan 12, 2025
@lnicola lnicola added the A-inlay-hints inlay/inline hints label Jan 12, 2025
@lnicola
Copy link
Member

lnicola commented Jan 12, 2025

the lower_inline_asm function has too many exit points for readers to distinguish the difference

What do you mean? I don't see any (early) exit points in there.

@Tunixer
Copy link
Author

Tunixer commented Jan 12, 2025

the lower_inline_asm function has too many exit points for readers to distinguish the difference

What do you mean? I don't see any (early) exit points in there.

There are too many implicit returns used in match statements, which do not have return keyword so that the reader cannot tell the exit point.

@lnicola
Copy link
Member

lnicola commented Jan 12, 2025

I still don't see them, but okay.

@Veykril
Copy link
Member

Veykril commented Jan 12, 2025

Can you give a concrete example? The only exit point in that mentioned function is

@Tunixer
Copy link
Author

Tunixer commented Jan 12, 2025

I still don't see them, but okay.

Image

As shown in the screenshot, there are too many implicit returns used in the nesting match statements, which would make the reader hard to identify which variable receives the implicit return value.

I think an inlay hint would be helpful to identify this kind of implicit returns.

@ChayimFriedman2
Copy link
Contributor

What exactly will the hint show?

@Tunixer
Copy link
Author

Tunixer commented Jan 13, 2025

What exactly will the hint show?

@Veykril @lnicola @ChayimFriedman2

I have designed a scheme to give a inlay hint to the reader, but it's my first time to issue a new feature request. Thus, it needs more discussion. I use two code snippets in crates\hir-def\src\body\lower\asm.rs as examples. Please review them and give me some suggestions.

Example 1

for piece in asm.asm_pieces() {
    let slot = operands.len();
    let mut ▶lower_reg = |reg: Option<ast::AsmRegSpec>| {
        let reg = reg?;
        if let Some(string) = reg.string_token() {
            reg_args.insert(slot);Some(InlineAsmRegOrRegClass::Reg(Symbol::intern(string.text())))
        } else {
            ⏎reg.name_ref().map(|name_ref| {
                InlineAsmRegOrRegClass::RegClass(Symbol::intern(&name_ref.text()))
            }) // The cursor is in this line.
        }
    };

Example 2

ast::AsmPiece::AsmOperandNamed(op) => {
      let name = op.name().map(|name| Symbol::intern(&name.text()));
      if let Some(name) = &name {
          named_args.insert(name.clone(), slot);
          named_pos.insert(slot, name.clone());
      }
      let Some(op) = op.asm_operand() else { continue };
      (
          name.map(Name::new_symbol_root),match op { // This is a rvalue, not a lvalue receiving the implicit return.
              ast::AsmOperand::AsmLabel(l) => {AsmOperand::Label(self.collect_block_opt(l.block_expr()))
              }
              ast::AsmOperand::AsmConst(c) => {AsmOperand::Const(self.collect_expr_opt(c.expr()))  // The cursor is in this line.
              }
              ast::AsmOperand::AsmSym(s) => {
                  let Some(path) =
                      s.path().and_then(|p| self.expander.parse_path(self.db, p))
                  else {
                      continue;
                  };AsmOperand::Sym(path)
              }
          },
      )
  }
};

When a user hovers and clicks the line of the implicit return in the nesting match block, the lsp would give inlay hints of before the clicked line and other implicit return lines in the same match block as shown in the code snippet. The user can find the variable assigned to the return value near the first tag and the match keyword.

When user changes the cursor to the other line, the tag is no longer hinted in the editor.

It is also helpful to add an comtemporary inlay hint of to the variable receiving the implicit return variable, also disappearing when the user moves the cursor to the other line.

@Tunixer Tunixer changed the title [feat] Optional Inlay Hint for Too Many Implicit Exit Points in One Function [feat] Optional Inlay Hint for Too Many Implicit Returns in One Function Jan 14, 2025
@lnicola
Copy link
Member

lnicola commented Jan 14, 2025

Too bad we can't draw lines in the editor like this:

Image

@Tunixer
Copy link
Author

Tunixer commented Jan 14, 2025

like this:

It it not clear about what's the relationship between the screenshot and this issue?

My plan does not include any drawn lines or any collapsing, expanding operations shown in the screenshot. It just adds some inlay hints of unicode symbols before some line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inlay-hints inlay/inline hints C-feature Category: feature request
Projects
None yet
Development

No branches or pull requests

4 participants