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

[Feature] Semantic highlighting for asm! and global_asm! strings? #6031

Closed
repnop opened this issue Sep 18, 2020 · 15 comments · Fixed by #18022
Closed

[Feature] Semantic highlighting for asm! and global_asm! strings? #6031

repnop opened this issue Sep 18, 2020 · 15 comments · Fixed by #18022
Labels
A-highlighting (semantic) token highlighting E-hard E-has-instructions Issue has some instructions and pointers to code to get started fun A technically challenging issue with high impact S-actionable Someone could pick this issue up and work on it right now

Comments

@repnop
Copy link
Contributor

repnop commented Sep 18, 2020

Hi, I was wondering if it would be possible to have inline syntax highlighting for raw literals that are used with the asm! and global_asm! macros via semantic highlighting, kind of like what happens with format string literals. Certainly wouldn't mind giving it a go myself if this is desired/possible :)

Thanks!

@lnicola
Copy link
Member

lnicola commented Sep 18, 2020

I can't give mentoring instructions, but it would probably be possible to implement this (and also awesome!). We have a couple of sub-languages supported: Rust code in docstrings and test fixtures and highlighting for format!-like macros. See crates/ide/src/syntax_highlighting.rs and crates/ide/src/syntax_highlighting/injection.rs.

@matklad matklad added E-hard E-has-instructions Issue has some instructions and pointers to code to get started fun A technically challenging issue with high impact S-actionable Someone could pick this issue up and work on it right now labels Oct 15, 2020
@matklad
Copy link
Member

matklad commented Oct 15, 2020

I can't give mentoring instructions,

Well, you just did :-)

@bjorn3
Copy link
Member

bjorn3 commented Oct 15, 2020

It would be nice if it also supported comments, labels and assembly directives highlighting. Those are almost the same across architectures with the exception of all comments other than //.

@matklad
Copy link
Member

matklad commented Oct 15, 2020

Hm, the syntax of asm is some specific llvm-assmebly syntax? I wonder if this yak shave should start with building and llvm_asm_syntax crate, a-la ra's rowan-based syntax? I think think it has a potential of being re-usable in various settings, not only in ra.

@lnicola
Copy link
Member

lnicola commented Oct 15, 2020

I think asm! is standardized (portable) and llvm_asm! is the old asm!.

@bjorn3
Copy link
Member

bjorn3 commented Oct 15, 2020

The actual asm in an asm! call is GNU assembly. Or at least LLVM is trying it's best to reproduce it for GCC compatibility.

@lnicola
Copy link
Member

lnicola commented Oct 15, 2020

I think the new syntax is closer to Intel? Talking about https://blog.rust-lang.org/inside-rust/2020/06/08/new-inline-asm.html and https://github.com/Amanieu/rfcs/blob/inline-asm/text/0000-inline-asm.md.

Reusing asm! was a bit confusing, I guess 😅.

@bjorn3
Copy link
Member

bjorn3 commented Oct 15, 2020

The new syntax is equivalent to wrapping the asm in .intel_syntax noprefix and .att_syntax. This is literally what I did when implementing asm for cg_clif using an external assembler

@repnop
Copy link
Contributor Author

repnop commented Oct 15, 2020

I thought about this a few times since I've made the issue but always got stuck on how to detect which flavor of assembly is being used inside of the macro, because the project I'm currently using asm! in is a RISC-V kernel but I imagine x86_64 dwarfs most other architectures except for perhaps ARM in terms of usage. Would it make sense for that to be something that you configure in the settings and select which assembly you're using since it's usually per-project? Otherwise, especially for small snippets, I feel like it can be difficult to differentiate exactly which one is being used (if that's even a problem) 🤔

@Diggsey
Copy link

Diggsey commented Oct 15, 2020

It's still an unresolved question whether the asm macro will be namespaced under the corresponding arch module or not.

If it is, then detecting the correct flavor should be fairly straightforward. If not, then rust-analyzer could attempt to infer that based on what cfg options are "live" at that point in the code.

@repnop
Copy link
Contributor Author

repnop commented Oct 15, 2020

Oh, I wasn't aware that was even being considered! I'll have to keep a closer eye on the tracking issue :) but that does sound pretty reasonable to discern if that does end up being the case 👍

@repnop
Copy link
Contributor Author

repnop commented Apr 2, 2021

I began writing a really rough version of this today as a proof-of-concept to see how hard it'd be, and was pleasantly surprised that it didn't take more than about 2 hours to get something working fairly well :) I'll probably put up a WIP PR sometime soon-ish for better discussion on the actual implementation.

question though, is there currently any way to query the target/architecture that the crate is being built for? my attempt right now is very general and doesn't assume a particular arch, but for more accurate highlighting that would be ideal. (assuming the asm! macro doesn't end up namespaced which would probably be better)

screenshot for the WIP for those who like pretty colors :D
image

@bstrie
Copy link
Contributor

bstrie commented Jul 6, 2021

(assuming the asm! macro doesn't end up namespaced which would probably be better)

Even a namespaced macro wouldn't guarantee that tools could determine from the source code which platforms a given asm invocation is intended for. You would still be capable of writing cross-platform asm! as follows:

#[cfg(target_arch="foo")]
use std::arch::foo::asm as foobar_asm;
#[cfg(target_arch="bar")]
use std::arch::bar::asm as foobar_asm;

foobar_asm!(...)

@matklad
Copy link
Member

matklad commented Jul 6, 2021

@bstrie rust-analyzer does precise analysis, so it has to analyze the code relative to a specific set of cfgs, and it knows what specific macro the call resolves to. At some point, we might implement the logic to union results of several active cfg sets (so we highlight the call twice and somehow merge the results), but we’ll never be able to be cfg agnostic.

@bstrie
Copy link
Contributor

bstrie commented Jul 6, 2021

Good to hear, although that still sounds like it's unaffected by whether or not the asm! macro ends up namespaced, which is my current concern since I'm proposing that the issue be brought up for a libs team decision. I'm currently not listing "would make syntax highlighting easier" as a point in favor of the pro-namespacing case, but if I'm wrong let me know.

@Veykril Veykril added the A-highlighting (semantic) token highlighting label Dec 4, 2021
@bors bors closed this as completed in d927daf Sep 5, 2024
lnicola pushed a commit to lnicola/rust that referenced this issue Sep 25, 2024
feat: IDE support for `asm!` expressions

Fixes rust-lang/rust-analyzer#10461, Fixes rust-lang/rust-analyzer#6031 Progresses rust-lang/rust-analyzer#11621

Notably this only works for asm expressions not items yet. Most IDE features work, mainly completions need extra logic still.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-highlighting (semantic) token highlighting E-hard E-has-instructions Issue has some instructions and pointers to code to get started fun A technically challenging issue with high impact S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants