================ @@ -450,6 +453,116 @@ class StmtComparer { }; } // namespace +static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, + const Attr *Attr1, const Attr *Attr2) { + // Two attributes are structurally equivalent if they are the same kind + // of attribute, spelled with the same spelling kind, and have the same + // arguments. This means that [[noreturn]] and __attribute__((noreturn)) are + // not structurally equivalent, nor are [[nodiscard("foo")]] and + // [[nodiscard("bar")]]. + if (Attr1->getKind() != Attr2->getKind()) + return false; + + if (Attr1->getSyntax() != Attr2->getSyntax()) + return false; + + if (Attr1->getSpellingListIndex() != Attr2->getSpellingListIndex()) + return false; + + auto GetAttrName = [](const Attr *A) { + if (const IdentifierInfo *II = A->getAttrName()) + return II->getName(); + return StringRef{}; + }; + + if (GetAttrName(Attr1) != GetAttrName(Attr2)) + return false; + + // FIXME: check the attribute arguments. Attr does not track the arguments on + // the base class, which makes this awkward. We may want to tablegen a + // comparison function for attributes? In the meantime, we're doing this the + // cheap way by pretty printing the attributes and checking they produce ---------------- AaronBallman wrote:
> The standard defines when things are compatible and it explicitly does not > require attributes to match for types to be compatible. Anything the standard doesn't talk about is UB by omission, and the standard makes no mention of attributes. There is nothing explicit about this situation. I want to dispense with this topic because I feel very strongly we move forward with the current approach. To make this more explicit as to why: 0) The goal of this feature is that two types which were compatible across TU boundaries are now compatible if defined within the same TU. So this is about naming and object layout, in general. Attributes impact these kinds of things (packed, transparent_union, type_visibility, preferred_name, randomize_layout, etc), and attribute arguments do as well (btf_decl_tag, objc_bridge_related, uuid, etc) 1) The ideal situation is we have individual attributes specify whether they do or do not make two types [in]compatible, then the equivalence checker can generically check this rather than requiring special logic that's trivial for us to forget when adding new attributes. 2) I'm not implementing the ideal solution because that's a significant undertaking, attributes are an edge case, and I only have so much bandwidth. I have a FIXME in the patch that makes this clear. 3) Since we're not getting the ideal solution today, we can either be safe by default (require attributes to be the same) or we can be unsafe by default (ignore attributes). I'm adamant we go with safe by default until the ideal solution can be implemented. https://github.com/llvm/llvm-project/pull/132939 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits