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

(c) atomic_init, atomic_xyz, etc. are all treated as types, but shouldn't be #3837

Open
Eisenwave opened this issue Aug 8, 2023 · 3 comments
Labels
bug help welcome Could use help from community language

Comments

@Eisenwave
Copy link
Contributor

Describe the issue
This rule is overly aggressive:

const TYPES = {
className: 'type',
variants: [
{ begin: '\\b[a-z\\d_]*_t\\b' },
{ match: /\batomic_[a-z]{3,6}\b/ }
]
};

As a result, atomic operations such as atomic_store, atomic_load, atomic_init, etc. are also highlighted as types. Instead of a general rule such as atomic_ being a type,

Which language seems to have the issue?
c

Are you using highlight or highlightAuto?
This is just based off of observations on Discord, which, to my knowledge uses highlight.js. Presumably highlight.

Sample Code to Reproduce

atomic_init()
atomic_load()
atomic_store()

Expected behavior
Only actual typedef-names such as atomic_int should be highlighted as types. Functions should not be highlighted as types.

Additional context
A list of types and functions can be found here https://en.cppreference.com/w/c/atomic. To fix this, it would not be sufficient to add negative lookahead to the pattern (to exclude function calls), since it is reasonable to write code such as:

// declare variable
auto x = std::atomic_int();
// type alias for a function returning atomic_int
using atomic_maker = atomic_int();

To avoid false positives, it would be better to add special cases for all the known atomic type aliases (there aren't that many anyway).

@Eisenwave Eisenwave added bug help welcome Could use help from community language labels Aug 8, 2023
@joshgoebel
Copy link
Member

Two easy paths:

  • Add a whitelist to the matcher to exclude the 3 common atomic functions (still wide, but may not matter)
  • Include a full list of atomic types

#2 seems easily doable for the short list I found following your link... I swear we've talked about this before... maybe double check cpp to see if we haven't already done this there and could just steal it.

@Eisenwave
Copy link
Contributor Author

#2 seems easily doable for the short list I found following your link... I swear we've talked about this before... maybe double check cpp to see if we haven't already done this there and could just steal it.

I've looked into it, and cpp doesn't seem to have any mechanism for treating atomic_int and such types specially. This kinda makes sense, because you usually write std::atomic_int or std::atomic<int>, there isn't much need for highlighting this one as a type in particular.

@Eisenwave
Copy link
Contributor Author

The whitelist would be quite short:
image

All other typedefs are covered by the rule for _t suffixed identifiers being highlighted as types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help welcome Could use help from community language
Projects
None yet
Development

No branches or pull requests

2 participants