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.

Sign up for GitHub

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

Jump to bottom

[Clang] Missing flag to allow using [[gnu::init_priority]] in source files #121108

Closed
philnik777 opened this issue Dec 25, 2024 · 7 comments · Fixed by #121577
Closed

[Clang] Missing flag to allow using [[gnu::init_priority]] in source files #121108

philnik777 opened this issue Dec 25, 2024 · 7 comments · Fixed by #121577
Assignees
@el-ev
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" extension:gnu good first issue https://github.com/llvm/llvm-project/contribute

Comments

@philnik777
Copy link
Contributor

Currently it's not possible to use [[gnu::init_priority(100)]] in source files except by using line control markers. It would be way nicer if there was a diagnostic group to disable this with the diagnostic being an error by default. GCC currently accepts this with a warning without a group instead, which is also annoying, but not as bad as making it only possible with a hack.
@github-actions github-actions bot added the clang Clang issues not falling into any other category label Dec 25, 2024
@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" extension:gnu and removed clang Clang issues not falling into any other category labels Dec 25, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 25, 2024

@llvm/issue-subscribers-clang-frontend

Author: Nikolas Klauser (philnik777)

Currently it's not possible to use `[[gnu::init_priority(100)]]` in source files except by using line control markers. It would be way nicer if there was a diagnostic group to disable this with the diagnostic being an error by default. GCC currently accepts this with a warning without a group instead, which is also annoying, but not as bad as making it only possible with a hack.

@shafik
Copy link
Collaborator

shafik commented Dec 27, 2024

CC @erichkeane

@erichkeane erichkeane added the good first issue https://github.com/llvm/llvm-project/contribute label Jan 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2025

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
  2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
  3. Fix the issue locally.
  4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  5. Create a Git commit.
  6. Run git clang-format HEAD~1 to format your changes.
  7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2025

@llvm/issue-subscribers-good-first-issue

Author: Nikolas Klauser (philnik777)

Currently it's not possible to use `[[gnu::init_priority(100)]]` in source files except by using line control markers. It would be way nicer if there was a diagnostic group to disable this with the diagnostic being an error by default. GCC currently accepts this with a warning without a group instead, which is also annoying, but not as bad as making it only possible with a hack.

@erichkeane
Copy link
Collaborator

Repro: https://godbolt.org/z/4T7ecxvjT

I think it is perfectly reasonable to emit the same warning (with a warning group of course) as GCC for this situation. This is likely a very simple fix, so I'll mark this as a good first issue. This should be a very simple addition of a diagnostic, then changing the handler to just emit the warning instead.

@el-ev
Copy link
Contributor

el-ev commented Jan 2, 2025

Hello, I'm new here and I would like to work on this.

@erichkeane
Copy link
Collaborator

Assigned! Please let me know if you have trouble with this, and let me know when you give up on this so others can take this.

@el-ev el-ev mentioned this issue Jan 3, 2025
Merged
AaronBallman pushed a commit that referenced this issue Mar 4, 2025
@el-ev
[clang] Fix gnu::init_priority attribute handling for reserved valu…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
Loading
Loading status checks…
9e1eaff 
…es (#121577)

- Added a new diagnostic group `InitPriorityReserved`
- Allow values within the range 0-100 of `init_priority` to be used
outside system library, but with a warning
- Updated relavant tests

Fixes #121108
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@el-ev el-ev

Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" extension:gnu good first issue https://github.com/llvm/llvm-project/contribute
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[clang] Fix gnu::init_priority attribute handling for reserved values el-ev/llvm-project
6 participants
@shafik @erichkeane @EugeneZelenko @philnik777 @llvmbot @el-ev