================
@@ -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
----------------
uecker 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 don't think this applies here.  We say that types are compatible under 
conditions A B and C. That we do not mention D does not mean there is UB. We 
also do not mention E, F and G or a billion other things. For me, an omission 
is when something is not precisely specified and you can reasonable interpret 
it either way. This is not the case here. But this only applies to standard 
attributes, vendor attributes can do whatever they want.
> 
> 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.

Sounds reasonable, but I would exempt standard attributes, because I believe 
this is what the standard requires.




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