Guard init_priority attribute within libc++

[zibi]

Summary

Not all platforms support priority attribute. I'm moving conditional definition of this attribute to include/__config.

[ldionne]

libcxx/include/__config

1430

You should use __has_attribute(init_priority).

[ldionne]

libcxx/include/__config

1430

Oh, and I would just define that in iostream.cpp if we don't use it anywhere else, to avoid adding complexity to __config (which is already a monster).

[zibi]

Thank you for review Louis. Please see inline comments.

libcxx/include/__config

1430

Oh, and I would just define that in iostream.cpp if we don't use it anywhere else, to avoid adding complexity to __config (which is already a monster).

Sure , this will require additional changes since that attribute is enabled by default and will have to be disabled at least for z/OS.
The init_priority is also used in experimental/memory_resource.cpp. Do we want to make changes there as well? I assume the main library and experimental don't share any common files so those guards will have to reside in 2 different places.

[ldionne]

libcxx/include/__config

1430

Interesting. In that case, I think it's fine to leave it in __config, however can you please also de-duplicate the usage in memory_resource.cpp?

[zibi]

libcxx/include/__config

1430

Sure, not sure if you still want to disable init_priority and use __has_attribute() guard. Please let me know before I investigate how to disable it.

[zibi]

I moved the logic from memory_resource.cpp to __config as since I'm not familiar with Apple and Microsoft restrictions are still correct.

[ldionne]

libcxx/include/experimental/__config

80 ↗

(On Diff #306162)

Just remove that entirely and use the one from the main <__config> file.

[zibi]

Moving macro to common place as requested. This makes assumption that MS and Apple restrictions were lifted at some point.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

384

This is not a very descriptive name -- if this is only supposed to be used for init_priority how about TargetSupportsInitPriority instead?

Also, this change is missing test coverage and documentation changes.

[zibi]

clang/include/clang/Basic/Attr.td

384

For now, we can use TargetSupportsInitPriority, we can always change it if we find the need to disable other attributes.

Thank you Arraon for review and suggestion for the name.

[zibi]

updated doc. and test as requested

[aaron.ballman]

clang/test/SemaCXX/init-priority-attr.cpp

28

Rather than using the preprocessor, you can assign a prefix to be checked to -verify. e.g., -verify=unknown would allow you to do // unknown-warning {{unknown attribute 'init_priority' ignored}} or unknown-no-diagnostics that is only checked when -verify=unknown.

I wonder if it would be cleaner to use that solution here instead of the preprocessor by adding a new RUN line that's specific to AIX, and setting some non-AIX triples for the other run lines.

[ldionne]

LGTM from libc++'s perspective. You should wait until the Clang part is LGTM'd before committing, of course.

[zibi]

Make test cleaner.

[aaron.ballman]

LGTM, thank you for the fixes!

[zibi]

clang/test/SemaCXX/init-priority-attr.cpp

28

That simplifies a lot in the expanse of duplicating the run.
Thank you Aaron.

clang/include/clang/Basic/Attr.td

 }
 def TargetMicrosoftCXXABI : TargetArch<["x86", "x86_64", "arm", "thumb", "aarch64"]> {
   let CustomCode = [{ Target.getCXXABI().isMicrosoft() }];
 }
 def TargetELF : TargetSpec {
   let ObjectFormats = ["ELF"];
 }
 
+def TargetSupportsInitPriority : TargetSpec {
+  let CustomCode = [{ !Target.getTriple().isOSzOS() }];
+}
 // Attribute subject match rules that are used for #pragma clang attribute.
 //
 // A instance of AttrSubjectMatcherRule represents an individual match rule.
 // An individual match rule can correspond to a number of different attribute
 // subjects, e.g. "record" matching rule corresponds to the Record and
 // CXXRecord attribute subjects.
 //
 // Match rules are used in the subject list of the #pragma clang attribute.
def WorkGroupSizeHint :  InheritableAttr {
   let Spellings = [GNU<"work_group_size_hint">];
   let Args = [UnsignedArgument<"XDim">,
               UnsignedArgument<"YDim">,
               UnsignedArgument<"ZDim">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [Undocumented];
 }
 
-def InitPriority : InheritableAttr {
+def InitPriority : InheritableAttr, TargetSpecificAttr<TargetSupportsInitPriority> {
   let Spellings = [GCC<"init_priority", /*AllowInC*/0>];
   let Args = [UnsignedArgument<"Priority">];
   let Subjects = SubjectList<[Var], ErrorDiag>;
   let Documentation = [InitPriorityDocs];
 }
 
 def Section : InheritableAttr {
   let Spellings = [GCC<"section">, Declspec<"allocate">];

clang/include/clang/Basic/AttrDocs.td

.. code-block:: c++
   struct SomeType { SomeType(); };
   __attribute__((init_priority(200))) SomeType Obj1;
   __attribute__((init_priority(101))) SomeType Obj2;
 
 ``Obj2`` will be initialized *before* ``Obj1`` despite the usual order of
 initialization being the opposite.
 
 This attribute is only supported for C++ and Objective-C++ and is ignored in
-other language modes.
+other language modes. Currently, this attribute is not implemented on z/OS.
   }];
 }
 
 def InitSegDocs : Documentation {
   let Category = DocCatVariable;
   let Content = [{
 The attribute applied by ``pragma init_seg()`` controls the section into
 which global initialization function pointers are emitted. It is only

clang/test/SemaCXX/init-priority-attr.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DSYSTEM -verify %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -fsyntax-only -DSYSTEM -verify %s
+// RUN: %clang_cc1 -triple=s390x-none-zos -fsyntax-only -verify=unknown %s
+// RUN: %clang_cc1 -triple=s390x-none-zos -fsyntax-only -DSYSTEM -verify=unknown-system %s
 
 #if defined(SYSTEM)
 #5 "init-priority-attr.cpp" 3 // system header
 #endif
 
 class Two {
 private:
     int i, j, k;
 public:
     static int count;
     Two( int ii, int jj ) { i = ii; j = jj; k = count++; };
     Two( void )           { i =  0; j =  0; k = count++; };
     int eye( void ) { return i; };
     int jay( void ) { return j; };
     int kay( void ) { return k; };
 };
 
 extern Two foo;
 extern Two goo;
 extern Two coo[];
 extern Two koo[];
 
 Two foo __attribute__((init_priority(101))) ( 5, 6 );
+ // unknown-system-no-diagnostics
+ // unknown-warning@-2 {{unknown attribute 'init_priority' ignored}}
 
 Two goo __attribute__((init_priority(2,3))) ( 5, 6 ); // expected-error {{'init_priority' attribute takes one argument}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 Two coo[2]  __attribute__((init_priority(100)));
 #if !defined(SYSTEM)
   // expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+  // unknown-warning@-3 {{unknown attribute 'init_priority' ignored}}
 #endif
 
 Two boo[2]  __attribute__((init_priority(65536)));
 #if !defined(SYSTEM)
  // expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+ // unknown-warning@-3 {{unknown attribute 'init_priority' ignored}}
 #endif
 
 Two koo[4]  __attribute__((init_priority(1.13))); // expected-error {{'init_priority' attribute requires an integer constant}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 Two func()  __attribute__((init_priority(1001))); // expected-error {{'init_priority' attribute only applies to variables}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
+
 
 int i  __attribute__((init_priority(1001))); // expected-error {{can only use 'init_priority' attribute on file-scope definitions of objects of class type}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 int main() {
-  Two foo __attribute__((init_priority(1001)));  // expected-error {{can only use 'init_priority' attribute on file-scope definitions of objects of class type}}
+  Two foo __attribute__((init_priority(1001))); // expected-error {{can only use 'init_priority' attribute on file-scope definitions of objects of class type}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 }

libcxx/include/__config

 
 // Support for _FILE_OFFSET_BITS=64 landed gradually in Android, so the full set
 // of functions used in cstdio may not be available for low API levels when
 // using 64-bit file offsets on LP32.
 #if defined(__BIONIC__) && defined(__USE_FILE_OFFSET64) && __ANDROID_API__ < 24
 #define _LIBCPP_HAS_NO_FGETPOS_FSETPOS
 #endif
 
+#if __has_attribute(init_priority)
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX
+#endif
+
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG

libcxx/src/experimental/memory_resource.cpp

struct {
     __new_delete_memory_resource_imp new_delete_res;
     __null_memory_resource_imp       null_res;
   } resources;
   char dummy;
   _LIBCPP_CONSTEXPR_AFTER_CXX11 ResourceInitHelper() : resources() {}
   ~ResourceInitHelper() {}
 };
 
-// Detect if the init_priority attribute is supported.
-#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
-  || defined(_LIBCPP_COMPILER_MSVC)
-// GCC on Apple doesn't support the init priority attribute,
-// and MSVC doesn't support any GCC attributes.
-# define _LIBCPP_INIT_PRIORITY_MAX
-#else
-# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
-#endif
-
 // When compiled in C++14 this initialization should be a constant expression.
 // Only in C++11 is "init_priority" needed to ensure initialization order.
 #if _LIBCPP_STD_VER > 11
 _LIBCPP_SAFE_STATIC
 #endif
 ResourceInitHelper res_init _LIBCPP_INIT_PRIORITY_MAX;
 
 } // end namespace

libcxx/src/iostream.cpp

 #endif
 ;
 _ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wclog[sizeof(wostream)]
 #if defined(_LIBCPP_ABI_MICROSOFT) && defined(__clang__)
 __asm__("?wclog@" _LIBCPP_ABI_NAMESPACE_STR "@std@@3V?$basic_ostream@_WU?$char_traits@_W@" _LIBCPP_ABI_NAMESPACE_STR "@std@@@12@A")
 #endif
 ;
 
-_LIBCPP_HIDDEN ios_base::Init __start_std_streams __attribute__((init_priority(101)));
+_LIBCPP_HIDDEN ios_base::Init __start_std_streams _LIBCPP_INIT_PRIORITY_MAX;
 
 // On Windows the TLS storage for locales needs to be initialized before we create
 // the standard streams, otherwise it may not be alive during program termination
 // when we flush the streams.
 static void force_locale_initialization() {
 #if defined(_LIBCPP_MSVCRT_LIKE)
   static bool once = []() {
     auto loc = newlocale(LC_ALL_MASK, "C", 0);