================
@@ -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

Reply via email to