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

1444

You should use __has_attribute(init_priority).

[ldionne]

libcxx/include/__config

1444

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

1444

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

1444

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

1444

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

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

26 ↗

(On Diff #306524)

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

26 ↗

(On Diff #306524)

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 ExcludeTarget : 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<ExcludeTarget> {
   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">];

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/include/experimental/__config

 
 // TODO: support more targets
 #if defined(__AVX__)
 #define _LIBCPP_NATIVE_SIMD_WIDTH_IN_BYTES 32
 #else
 #define _LIBCPP_NATIVE_SIMD_WIDTH_IN_BYTES 16
 #endif
 
+// Detect if the init_priority attribute is supported.
+#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
+  || defined(_LIBCPP_COMPILER_MSVC) || defined(__MVS__)
+// GCC on Apple doesn't support the init priority
+// attribute, same for MSVC and z/OS.
+# define _LIBCPP_INIT_PRIORITY_MAX
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#endif
+
 #endif

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

 //===------------------------ iostream.cpp --------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "__std_stream"
 #include "__locale"
 #include "string"
 #include "new"
 
 #define _str(s) #s
 #define str(s) _str(s)
 #define _LIBCPP_ABI_NAMESPACE_STR str(_LIBCPP_ABI_NAMESPACE)
 

 #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);