vsapsai marked 3 inline comments as done.
vsapsai added a comment.
Given all the discussion about which attributes should be added to ODR hash, I
think it is useful at this point to have TableGen infrastructure to get this
information from Attr.td. So I'll work on that.
================
Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:159
bool ODRDiagsEmitter::diagnoseSubMismatchTypedef(
const NamedDecl *FirstRecord, StringRef FirstModule, StringRef
SecondModule,
----------------
vsapsai wrote:
> aaron.ballman wrote:
> > Typedefs can have attributes, so should this be updated as well?
> > (alignment, ext vector types, mode attributes, etc all come to mind)
> It should be tested for sure, thanks for pointing it out.
Typedefs are done. But while adding support for them I've realized we don't
check method parameters. So it requires a little bit more work.
================
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:
> vsapsai wrote:
> > 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)`
> > ?
> Sorry for the ambiguity. Since `LHS->getKind() != RHS->getKind()` is covered
> by `ComputeAttrODRHash(LHS) != ComputeAttrODRHash(RHS)`. I feel like it is
> reasonable to:
>
> ```
> if (!LHS || !RHS || ComputeAttrODRHash(LHS) != ComputeAttrODRHash(RHS)) {
> DiagError();
> DiagNote();
> DiagNote();
> DiagNoteAttrLoc();
> return true;
> }
> ```
There are 2 separate cases to improve the diagnostic. In the first case we'd
have
```
... first difference is definition in module 'FirstModule' found attribute
'enum_extensibility'
attribute specified here
but in 'SecondModule' found no attribute
```
And if we reach the second ones, it implies the kind of attributes is the same
and the only difference is attribute arguments, so the diagnostics are like
```
first difference is definition in module 'FirstModule' found attribute '
__attribute__((enum_extensibility("open")))'
attribute specified here
but in 'SecondModule' found different attribute argument '
__attribute__((enum_extensibility("closed")))'
attribute specified here
```
From my limited experience, it is actually useful to have more details than
trying to figure out the difference in attributes.
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
