AaronBallman wrote:
> > > > > I concluded that the local queue introduced in the previous commits
> > > > > isn’t necessary in C mode. Since attribute equivalence is currently
> > > > > checked only for C23, I removed the code entirely.
> > > > > If we later enable attribute equivalence checking in C++ mode, we may
> > > > > need to revisit this and adopt a similar approach. For example,
> > > > > without a separate queue, clang will diagnose the attribute but fail
> > > > > to report the mismatch in the type of member `a` when attempting to
> > > > > merge the following struct:
> > > > > ```
> > > > > struct GuardedBy {
> > > > > int b __attribute__((guarded_by(lock)));
> > > > > // The pair of S is added to NonEquivalentDecls, but the pair of
> > > > > Lock isn't.
> > > > > struct __attribute__ ((lockable)) Lock {
> > > > > struct S {
> > > > > #ifdef D1
> > > > > unsigned a;
> > > > > #else
> > > > > int a;
> > > > > #endif
> > > > > } s;
> > > > > } lock;
> > > > > };
> > > > > ```
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > $ clang -cc1 -std=c++20 -emit-pch -o f1.ast -x c++ test.h $ clang
> > > > > -cc1 -std=c++20 -emit-pch -o f2.ast -DD1 -x c++ test.h $ clang -cc1
> > > > > -std=c++20 -ast-merge f1.ast -ast-merge f2.ast -x c++ test.cpp
> > > >
> > > >
> > > > CC @ChuanqiXu9 @Bigcheese as C++ modules maintainers. I am under the
> > > > impression we need this for both C++ modules support and C23's
> > > > redefinition checking. If we don't need the more complicated approach
> > > > right now, I think the current approach is reasonable for C (but CC
> > > > @erichkeane for awareness of the tablegen changes), I just want to make
> > > > sure we're not making further work harder for ourselves.
> > >
> > >
> > > In modules (Serialization), we already have ODR checks to check the
> > > redefinitions are the same. So this is not needed for modules.
> >
> >
> > Can you point me to the code that's handling checking attribute
> > compatibility?
>
> Oh, you're talking about attribute. I was thought you're talking about the
> compatibility of struct fields (according to the code example you mentioned).
>
> For attribute, now we don't check the compatibility. We can have a
> declaration chain with redeclarations have different attributes now.
That's what this PR is doing, but it's currently only doing it for C and not
for C++. Because we need the facilities for both C and C++ and the logic should
be basically identical between the languages (I don't know of any attributes
supported in both languages which impact layout or semantics in one language
but do not in the other). I thought that modules was using the AST structural
equivalence code as well; if you confirm, do you think we should do the C++
changes as well, either in this PR or a follow-up? Or is there a better way for
us to structure the code for sharing?
https://github.com/llvm/llvm-project/pull/168769
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
