-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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++: Support more "noreturn" attributes in DefaultOptions #18510
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 4 changed files in this pull request and generated no comments.
Files not reviewed (3)
- cpp/ql/lib/DefaultOptions.qll: Language not supported
- cpp/ql/test/query-tests/jsf/4.13 Functions/AV Rule 114/test.c: Language not supported
- cpp/ql/test/query-tests/jsf/4.13 Functions/AV Rule 114/test.cpp: Language not supported
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
f18(); // GOOD | ||
} | ||
|
||
[[___Noreturn__]] void f20(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's going on with __Noreturn__
(in the change note), ___Noreturn__
(here, with the extra _
), and the predicate itself (which list neither)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be with just two _
, well spotted. Let me temporarily put this back in draft and fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wrong. It should be three _
as a prefix and two as a suffix, so it's just the change note that's incorrect. See: https://en.cppreference.com/w/c/language/attributes/noreturn. Note that the lower case version just has two _
both as prefix and as suffix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The predicate itself does not list ___Noreturn__
, as the frontend strips off two of the _
in the prefix and also strips off the __
suffix. So, those do not end up in the database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, they've made this unnecessarily fiddly!
Pull Request checklist
All query authors
.qhelp
. See the documentation in this repository.Internal query authors only
.ql
,.qll
, or.qhelp
files. See the documentation (internal access required).