[ADT] Mark `llvm::array_lengthof` as deprecated

[jloser]

Summary

As a follow-up of 5e96cea1db0623a833d5376c9ea2ce4528771f97, mark
llvm::array_lengthof as deprecated in favor of using std::size function
directly.

[kazu]

As I commented in D133429, please be sure to take care of replacements in internal downstream users before checking this in. LGTM other than that. Thanks!

llvm/include/llvm/ADT/STLArrayExtras.h

31–32

Consider saying something like:

LLVM_DEPRECATED("Use std::size instead.", "std::size")

This way, the compiler warning 'underlines' array_lengthof, which I find very helpful.

[kazu]

By the way, I just realized that array_lengthof is the only thing in STLArrayExtras.h. You might want to consider a follow-up patch to stop including the header file (at 7 places or so). Thanks!

[jloser]

Use LLVM_DEPRECATED instead

[jloser]

In D133502#3777484, @kazu wrote:

By the way, I just realized that array_lengthof is the only thing in STLArrayExtras.h. You might want to consider a follow-up patch to stop including the header file (at 7 places or so). Thanks!

Yep, I plan on doing that and fixing clang and lldb to no longer use this function as well. Once all of the internal callers are using std::size, then I'll land this.

llvm/include/llvm/ADT/STLArrayExtras.h

31–32

Just changed to using LLVM_DEPRECATED — thanks!

[MaskRay]

With my downstream inspection, I think array_lengthof is used much less than Optional::{hasValue,getValue}, etc.
I appreciate that Optional::{hasValue,getValue} had a very long deprecation period so downstream can be more prepared.
array_lengthof removal may not need that long time.

[jloser]

Placement of LLVM_DEPRECATED matters

[jloser]

Include "llvm/Support/Compiler.h" explicitly for using LLVM_DEPRECATED

[mstorsjo]

This produces a warning when built with GCC for every single source file that includes the header:

../include/llvm/Support/Compiler.h:145:35: warning: attribute ignored [-Wattributes]  
  145 | #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]]
      |                                   ^
../include/llvm/ADT/STLArrayExtras.h:31:18: note: in expansion of macro ‘LLVM_DEPRECATED’        
   31 | constexpr inline LLVM_DEPRECATED("Use std::size instead.", "std::size") size_t
      |                  ^~~~~~~~~~~~~~~
../include/llvm/Support/Compiler.h:145:35: note: an attribute that appertains to a type-specifier is ignored              
  145 | #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]]
      |                                   ^
../include/llvm/ADT/STLArrayExtras.h:31:18: note: in expansion of macro ‘LLVM_DEPRECATED’
   31 | constexpr inline LLVM_DEPRECATED("Use std::size instead.", "std::size") size_t
      |                  ^~~~~~~~~~~~~~~

[lattner]

nice, thanks Joe!

[MaskRay]

In D133502#3781560, @mstorsjo wrote:

This produces a warning when built with GCC for every single source file that includes the header:

../include/llvm/Support/Compiler.h:145:35: warning: attribute ignored [-Wattributes]  
  145 | #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]]
      |                                   ^
../include/llvm/ADT/STLArrayExtras.h:31:18: note: in expansion of macro ‘LLVM_DEPRECATED’        
   31 | constexpr inline LLVM_DEPRECATED("Use std::size instead.", "std::size") size_t
      |                  ^~~~~~~~~~~~~~~
../include/llvm/Support/Compiler.h:145:35: note: an attribute that appertains to a type-specifier is ignored              
  145 | #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]]
      |                                   ^
../include/llvm/ADT/STLArrayExtras.h:31:18: note: in expansion of macro ‘LLVM_DEPRECATED’
   31 | constexpr inline LLVM_DEPRECATED("Use std::size instead.", "std::size") size_t
      |                  ^~~~~~~~~~~~~~~

Should be fixed now. The placement before Placement of LLVM_DEPRECATED matters was actually correct for [[deprecated(...)]] :)

[jloser]

In D133502#3781664, @MaskRay wrote:

In D133502#3781560, @mstorsjo wrote:

This produces a warning when built with GCC for every single source file that includes the header:

../include/llvm/Support/Compiler.h:145:35: warning: attribute ignored [-Wattributes]  
  145 | #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]]
      |                                   ^
../include/llvm/ADT/STLArrayExtras.h:31:18: note: in expansion of macro ‘LLVM_DEPRECATED’        
   31 | constexpr inline LLVM_DEPRECATED("Use std::size instead.", "std::size") size_t
      |                  ^~~~~~~~~~~~~~~
../include/llvm/Support/Compiler.h:145:35: note: an attribute that appertains to a type-specifier is ignored              
  145 | #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]]
      |                                   ^
../include/llvm/ADT/STLArrayExtras.h:31:18: note: in expansion of macro ‘LLVM_DEPRECATED’
   31 | constexpr inline LLVM_DEPRECATED("Use std::size instead.", "std::size") size_t
      |                  ^~~~~~~~~~~~~~~

Should be fixed now. The placement before Placement of LLVM_DEPRECATED matters was actually correct for [[deprecated(...)]] :)

Thanks for the quick fix, @MaskRay — I appreciate it. By the way, the extra includes for STLArrayExtras.h in the LLVM tree is addressed with https://reviews.llvm.org/D133600

llvm/include/llvm/ADT/STLArrayExtras.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ADT_STLARRAYEXTRAS_H
 #define LLVM_ADT_STLARRAYEXTRAS_H
 
 #include <cstddef>
 
+#include "llvm/Support/Compiler.h"
+
 namespace llvm {
 
 //===----------------------------------------------------------------------===//
 //     Extra additions for arrays
 //===----------------------------------------------------------------------===//
 
 /// Find the length of an array.
 template <class T, std::size_t N>
-constexpr inline size_t array_lengthof(T (&)[N]) {
+constexpr inline LLVM_DEPRECATED("Use std::size instead.", "std::size") size_t
+    array_lengthof(T (&)[N]) {
   return N;
 }
 
 } // end namespace llvm
 
 #endif // LLVM_ADT_STLARRAYEXTRAS_H