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

(llvm) types and blocks are parsed incorrectly #2782

Closed
5 of 8 tasks
gnzlbg opened this issue Oct 23, 2020 · 8 comments · Fixed by #2830
Closed
5 of 8 tasks

(llvm) types and blocks are parsed incorrectly #2782

gnzlbg opened this issue Oct 23, 2020 · 8 comments · Fixed by #2830
Labels
bug good first issue Should be easier for first time contributors help welcome Could use help from community language

Comments

@gnzlbg
Copy link

gnzlbg commented Oct 23, 2020

Describe the issue

The LLVM types and blocks are parsed incorrectly.

Which language seems to have the issue?
LLVM

Are you using highlight or highlightAuto?
No

Sample Code to Reproduce

;; foooo
define i32 @mul_add(i32 %x, i32 %y, i32 %z) {
  entry:
    %tmp = mul i32 %x, %y
    %tmp2 = add i32 %tmp, %z
    %tmp3 = add i32 %tmp, 0
    ret i32 %tmp3
}

Expected behavior
I expect:

  • define to be a .hljs-keyword
  • i32 to be a .hljs-type (its parsed as a keyword, all types are described here: https://llvm.org/docs/LangRef.html#type-system)
  • @mul_add to be a .hljs-title (its a function name)
  • all the %... to be .hljs-symbol
  • 0 to be a .hljs-literal
  • mul, add, ret to be .hljs-built_in or .hljs-keyword (these are parsed as keywords)
  • entry: is a block, so i'd expect this to be maybe a .hljs-section ? not sure
  • ;; fooo to be a comment
@gnzlbg gnzlbg added bug help welcome Could use help from community language labels Oct 23, 2020
@joshgoebel joshgoebel added the good first issue Should be easier for first time contributors label Oct 23, 2020
@joshgoebel
Copy link
Member

joshgoebel commented Oct 23, 2020

Why is 0 a literal and not a number? literal is typically reserved for built-in literal values in a language, ie: NULL, true, false, etc...

Tagging beginner friendly.

  • We don't really match types now at all over than i* (as keyword)... I assume that rule was added to match i32, i8, i64, etc... so it'd be nice to have an explicit types list (for accuracy if nothing else).
  • For the label I'd suggest meta I think. In other languages we use symbol but we're using that for "%identifier"... are these symbols or variables? If they are variables we could just use variable and then use symbol for the labels.

Many styles make almost a "subtheme" for Markdown (which is most often where section is used) which can make it a poor choice to general programing languages. We are working on more semantic descriptors for things though but that's a bit of a long-term mission...

Are these truly called "blocks" in LLVM?

@joshgoebel
Copy link
Member

I see string are permittable (in the grammar)... is an empty string allowed? ie, ""?

@joshgoebel
Copy link
Member

Ping.

1 similar comment
@joshgoebel
Copy link
Member

Ping.

@allejo
Copy link
Member

allejo commented Nov 13, 2020

The closest thing I can find about 0 being a "literal" is when it's preceded by a !...? For example, in llvm's "Module Structure" example snippet.

; Named metadata
!0 = !{i32 42, null, !"string"}
!foo = !{!0}

With PR #2830, I see the following difference in our highlight:

image

@joshgoebel
Copy link
Member

joshgoebel commented Nov 13, 2020

Thanks for that example. That looks wrong. I’m also not sure I’d call that a literal though. But it seems perhaps the exclamation rule should come before the number rule.

; Some unnamed metadata nodes, which are referenced by the named metadata.
!0 = !{!"zero"}
!1 = !{!"one"}
!2 = !{!"two"}
; A named metadata.
!name = !{!0, !1, !2}

@allejo
Copy link
Member

allejo commented Nov 13, 2020

Yea, I wouldn't call it a literal either. That usage is the closest thing I could find to it being considered something other than a number.

@joshgoebel
Copy link
Member

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue Should be easier for first time contributors help welcome Could use help from community language
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants