[Sema] Ignore transparent_union attributes in C++

[arphaman]

Summary

While working on fixing PR30520 yesterday I noticed that clang also crashes on code like this:

template<typename T>
union u { T b; } __attribute__((transparent_union));

I mentioned in https://reviews.llvm.org/D25273 that I plan on fixing it properly by instantiating the attribute, but, after looking more into it, I discovered that we don't really support 'transparent_union' in C++ at all. This is why I decided to go with the approach that's presented in this patch: the attribute is completely ignored when clang is in C++ mode, and thus the crash is avoided.

I wasn't sure if we should we emit an error or a warning when ignoring this attribute and if we need any diagnostics in Gnu mode at all. That's why in this initial patch doesn't emit a diagnostic when ignoring this attribute. Please let me know whether we need to show one or not.

Thanks

[aaron.ballman]

lib/Sema/SemaDeclAttr.cpp

3011

The correct way to do this is to mark the attribute as C-only on its declaration in Attr.td. See the definition of FlagEnum for an example.

[arphaman]

The updated patch uses a proper way to mark the attribute as C only.

[arphaman]

The updated patch now makes clang warn every time it encounters this attribute in C++ mode. Would that be the desired behaviour?

[hfinkel]

In D25308#566176, @arphaman wrote:

The updated patch now makes clang warn every time it encounters this attribute in C++ mode. Would that be the desired behaviour?

As I understand it, transparent_union was designed for use in system headers, and these might certainly be included into C++ source files. Does that not work correctly, or does the feature just not work correctly when combined with C++-specific features (templates, function overloading, etc.)?

[aaron.ballman]

In D25308#566176, @arphaman wrote:

The updated patch now makes clang warn every time it encounters this attribute in C++ mode. Would that be the desired behaviour?

Yes, I believe that is the correct behavior in this instance.

The patch LGTM now, thank you!

include/clang/Basic/Attr.td

1545 ↗

(On Diff #74127)

If you, or anyone else, is bored, it would be nice to document this attribute now that we're touching it. (I am not signing you up for work as part of this patch, just prodding in case you wanted to do it.)

[arphaman]

Thanks!

include/clang/Basic/Attr.td

1545 ↗

(On Diff #74127)

That's a good idea, the lack of documentation stood out to me as well. I'll try to find some time this week for it.

lib/Sema/SemaDeclAttr.cpp

FormatAttr *NewAttr = S.mergeFormatAttr(D, Attr.getRange(), II,
                                           Idx, FirstArg,
                                           Attr.getAttributeSpellingListIndex());
   if (NewAttr)
     D->addAttr(NewAttr);
 }
 
 static void handleTransparentUnionAttr(Sema &S, Decl *D,
                                        const AttributeList &Attr) {
+  if (S.getLangOpts().CPlusPlus) {
+    return;
+  }
+
   // Try to find the underlying union declaration.
   RecordDecl *RD = nullptr;
   TypedefNameDecl *TD = dyn_cast<TypedefNameDecl>(D);
   if (TD && TD->getUnderlyingType()->isUnionType())
     RD = TD->getUnderlyingType()->getAsUnionType()->getDecl();
   else
     RD = dyn_cast<RecordDecl>(D);
 

test/SemaCXX/attr-gnu.cpp

 
 class D : public B {
 public:
   void test() __attribute__((deprecated)) final {} // expected-warning {{GCC does not allow an attribute in this position on a function declaration}}
   void test2() [[]] override {} // Ok
   void test3() __attribute__((cf_unknown_transfer)) override {} // Ok, not known to GCC.
 };
 }
+
+template<typename T>
+union Tu { T b; } __attribute__((transparent_union));
+
+template<typename T>
+union Tu2 { int x; T b; } __attribute__((transparent_union));
+
+union Tu3 { int x; } __attribute((transparent_union));
+
+void tuTest1(Tu<int> u); // expected-note {{candidate function not viable: no known conversion from 'int' to 'Tu<int>' for 1st argument}}
+void tuTest2(Tu3 u); // expected-note {{candidate function not viable: no known conversion from 'int' to 'Tu3' for 1st argument}}
+void tu() {
+  int x = 2;
+  tuTest1(x); // expected-error {{no matching function for call to 'tuTest1'}}
+  tuTest2(x); // expected-error {{no matching function for call to 'tuTest2'}}
+}