[PATCH] D144583: [ADT] Fix definition of `adl_begin`/`adl_end` and `Iter`/`ValueOfRange`

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

In D144583#4145554 <https://reviews.llvm.org/D144583#4145554>, @dblaikie wrote:

>> Fix adl_begin/adl_end so that they work with incomplete types. These require trailing return types, per. 7.5.2.: > In a trailing-return-type, the class being defined is not required to be complete for purposes of class member access. Class members declared later are not visible.
>
> Not sure I follow here - could you describe this in more detail, why adding a trailing return type helps? What happens if the trailing return type isn't used here?

If you return just `auto`, it fails to compile code like:

  template <typename R, typename T = ValueOfRange<R>>
  void foo(R&&) { ...

if R is an incomplete type and it cannot lookup the definition of `.begin()` inside `std::begin`.

We rely on this in a few places, e.g., `llvm::interleave` or `mlir::Diagnostic::operator<<`:

  class Operation;
  ...
  
  /// Stream in a range.
  template <typename T, typename ValueT = llvm::detail::ValueOfRange<T>>
  std::enable_if_t<!std::is_constructible<DiagnosticArgument, T>::value,
                   Diagnostic &>
  operator<<(T &&range) {
    return appendRange(range);
  }

Error:

  /usr/local/google/home/kubak/llvm/llvm-project/llvm/include/llvm/ADT/STLExtras.h:68:10: error: no matching function for call to 'begin'                              
    return begin(container);                                                                                                                                           
           ^~~~~                                                                                                                                                       
  /usr/local/google/home/kubak/llvm/llvm-project/llvm/include/llvm/ADT/STLExtras.h:90:39: note: in instantiation of function template specialization 'llvm::adl_begin<c
  onst char *&>' requested here                                                                                                                                        
      std::remove_reference_t<decltype(*adl_begin(std::declval<RangeT &>()))>;                                                                                         
                                        ^                                                                                                                              
  /usr/local/google/home/kubak/llvm/llvm-project/mlir/include/mlir/IR/Diagnostics.h:210:57: note: in instantiation of template type alias 'ValueOfRange' requested here
    template <typename T, typename ValueT = llvm::detail::ValueOfRange<T>>                                                                                             
                                                          ^                                                                                                            
  /usr/local/google/home/kubak/llvm/llvm-project/mlir/include/mlir/IR/Diagnostics.h:213:3: note: in instantiation of default argument for 'operator<<<const char *&>' r
  equired here                                                                                                                                                         
    operator<<(T &&range) {                                                                                                                                            
    ^~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                            
  /usr/local/google/home/kubak/llvm/llvm-project/mlir/include/mlir/IR/Diagnostics.h:222:65: note: while substituting deduced template arguments into function template 
  'operator<<' [with T = const char *&, ValueT = (no value)]                                                                                                           
          c, [this](const auto &a) { *this << a; }, [&]() { *this << delim; });                                                                                        
                                                                  ^                                                                                                    
  /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/range_access.h:95:5: note: candidate template ignored: could not match '_Tp[_Nm]' against 'co
  nst char *'                                                                                                                                                          
      begin(_Tp (&__arr)[_Nm]) noexcept                                                                                                                                
      ^                                                                                                                                                                
  /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/range_access.h:113:31: note: candidate template ignored: could not match 'valarray<_Tp>' agai
  nst 'const char *'                                                                                                                                                   
    template<typename _Tp> _Tp* begin(valarray<_Tp>&) noexcept;                                                                                                        
                                ^                                                                                                                                      
  /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/range_access.h:114:37: note: candidate template ignored: could not match 'valarray<_Tp>' agai
  nst 'const char *'                                                                                                                                                   
    template<typename _Tp> const _Tp* begin(const valarray<_Tp>&) noexcept;                                                                                            
                                      ^                                                                                                                                
  /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/range_access.h:52:5: note: candidate template ignored: substitution failure [with _Container 
  = const char *]: member reference base type 'const char *' is not a structure or union                                                                               
      begin(_Container& __cont) -> decltype(__cont.begin())                                                                                                            
      ^                                           ~                                                                                                                    
  /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/range_access.h:63:5: note: candidate template ignored: substitution failure [with _Container 
  = const char *]: member reference base type 'const char *const' is not a structure or union                                                                          
      begin(const _Container& __cont) -> decltype(__cont.begin())                                                                                                      
      ^                                                 ~                                                                                                              
  /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/initializer_list:90:5: note: candidate template ignored: could not match 'initializer_list<_Tp>' a
  gainst 'const char *'                                                                                                                                                
      begin(initializer_list<_Tp> __ils) noexcept    

I'm not sure how to describe this best.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144583/new/

https://reviews.llvm.org/D144583