-
Notifications
You must be signed in to change notification settings - Fork 7
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
cheri_compartment: Warn if return type is void or return value is unused #79
Conversation
The current implementation has a little shortcoming: the warning message for void return type is shown twice because cheri_compartment is a function type attribute (cf SemaType.cpp) that is added to both the attribute list of the declarator and the one of the function type.
Changes all look ok to you? |
static void handleCHERICompartmentName(Sema &S, Decl *D, const ParsedAttr &Attr) { | ||
static void handleCHERICompartmentName(Sema &S, Decl *D, const ParsedAttr &Attr, | ||
Sema::DeclAttributeLocation DAL) { | ||
// cheri_compartment is both: |
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 think we need to handle the attribute only at the DeclSpec level and not in the DeclChunk one. Otherwise, it generates duplicate messages.
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.
Oh that's the part that lead to the linking error ! I'll try to recreate the function declaration/definition where the attribute is skipped by this early return.
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 think I've got the gotcha. An attribute after a pointer return type remains tied to the DeclChunk (void * __attribute__((...))
). So my code was wrong.
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.
maybe I should do the way round, ie only process the DeclChunks.
Feel free to pull this branch into your repo and make your own PR with the correct version. |
Reapply #47
Changes since that version: