vsapsai added a comment.
In D135472#3844688 <https://reviews.llvm.org/D135472#3844688>, @ChuanqiXu wrote:
> I guess the reason why we didn't check odr violation for attributes is that
> the attributes was not a part of declarations in C++ before. But it is odd to
> skip the check for attributes from the perspective of users. So I think this
> one should be good.
I think we haven't seen many problems with attribute mismatches because in
existing code attributes are often hidden behind macros. And we have sugar
`MacroQualifiedType` that causes the definitions
#define NODEREF __attribute__((noderef))
struct StructWithField {
int NODEREF *i_ptr;
};
struct StructWithField {
int __attribute__((noderef)) *i_ptr;
};
to be treated as incompatible.
But the keyword `alignas` is used without macros and more attributes can be
used in multiple compilers without macros. That's why I think it is useful to
detect and to diagnose mismatches in attributes.
In D135472#3844688 <https://reviews.llvm.org/D135472#3844688>, @ChuanqiXu wrote:
> The only concern is that it misses too many checks for attributes. Do you
> plan to add it in near future or in longer future?
In the near future I was planning to add various Objective-C and Swift
attributes. For other attributes I don't know which are high-value. I
definitely want to check attributes affecting the memory layout (alignment,
packing) and believe I've addressed them (totally could have missed something).
In D135472#3844688 <https://reviews.llvm.org/D135472#3844688>, @ChuanqiXu wrote:
> And I guess we need to mention this in ReleaseNotes in `Potential Breaking
> Changes` section.
Good idea, will do.
================
Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:337-354
+ if (!LHS || !RHS || LHS->getKind() != RHS->getKind()) {
+ DiagError(AttributeKind)
+ << (LHS != nullptr) << LHS << FirstDecl->getSourceRange();
+ DiagNoteAttrLoc(LHS);
+ DiagNote(AttributeKind)
+ << (RHS != nullptr) << RHS << SecondDecl->getSourceRange();
+ DiagNoteAttrLoc(RHS);
----------------
ChuanqiXu wrote:
> I feel like we can merge these two statements.
Sorry, I don't really get what two statements you are talking about. Is it
* `!LHS || !RHS || LHS->getKind() != RHS->getKind()`
* `ComputeAttrODRHash(LHS) != ComputeAttrODRHash(RHS)`
?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135472/new/
https://reviews.llvm.org/D135472
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
