[PATCH] D102760: [llvm] Let SmallVector construct from any Iterable

Thu Jun 3 00:37:24 PDT 2021

courbet added inline comments.

================
Comment at: clang/include/clang/Basic/Module.h:98
 public:
+  Module(const Module &) = default;
+
----------------
gchatelet wrote:
> gchatelet wrote:
> > fhahn wrote:
> > > how is this related?
> > > how is this related?
> > 
> > I'm glad you ask! I'm still investigating the errors that led to this addition (and the one in `llvm/tools/llvm-xray/xray-converter.cpp`) but I suspect this is due to the recursive nature of these classes.
> > Both `Module` and `StackIdData` contains self-referencing `SmallVector`. It is unclear to me why the compiler is trying to instantiate the newly added `SmallVector` constructor //during// type declaration.
> > 
> > I'll post the compiler errors below the additions.
> ```
> FAILED: tools/clang/lib/Lex/CMakeFiles/obj.clangLex.dir/HeaderSearch.cpp.o 
> /redacted/build-llvm/download/clang/bin/clang++ --sysroot=/redacted/build-llvm/download/sysroot -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/lib/Lex -I/redacted/git/llvm-project/clang/lib/Lex -I/redacted/git/llvm-project/clang/include -Itools/clang/include -Iinclude -I/redacted/git/llvm-project/llvm/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG  -fno-exceptions -fno-rtti -std=c++14 -MD -MT tools/clang/lib/Lex/CMakeFiles/obj.clangLex.dir/HeaderSearch.cpp.o -MF tools/clang/lib/Lex/CMakeFiles/obj.clangLex.dir/HeaderSearch.cpp.o.d -o tools/clang/lib/Lex/CMakeFiles/obj.clangLex.dir/HeaderSearch.cpp.o -c /redacted/git/llvm-project/clang/lib/Lex/HeaderSearch.cpp
> In file included from /redacted/git/llvm-project/clang/lib/Lex/HeaderSearch.cpp:13:
> In file included from /redacted/git/llvm-project/clang/include/clang/Lex/HeaderSearch.h:16:
> In file included from /redacted/git/llvm-project/clang/include/clang/Basic/SourceLocation.h:19:
> /redacted/git/llvm-project/llvm/include/llvm/Support/PointerLikeTypeTraits.h:61:28: error: invalid application of 'alignof' to an incomplete type 'clang::Module'
>       detail::ConstantLog2<alignof(T)>::value;
>                            ^~~~~~~~~~
> /redacted/git/llvm-project/llvm/include/llvm/ADT/IterableTraits.h:23:33: note: in instantiation of template class 'llvm::PointerLikeTypeTraits<clang::Module *>' requested here
>     std::next(std::declval<I>()),
>                                 ^
> /redacted/git/llvm-project/llvm/include/llvm/ADT/IterableTraits.h:36:39: note: while substituting explicitly-specified template arguments into function template 'is_forward_iterator' 
> struct is_forward_iterator : decltype(detail::is_forward_iterator<I>(0)) {};
>                                       ^
> /redacted/git/llvm-project/llvm/include/llvm/ADT/IterableTraits.h:46:42: note: in instantiation of template class 'llvm::is_forward_iterator<const llvm::PointerIntPair<clang::Module *, 1, bool> *>' requested here
>                                          llvm::is_forward_iterator<I>{});
>                                          ^
> /redacted/git/llvm-project/llvm/include/llvm/ADT/IterableTraits.h:51:37: note: while substituting deduced template arguments into function template 'is_range_iterable' [with T = const llvm::SmallVector<llvm::PointerIntPair<clang::Module *, 1, bool>, 2> &, I = (no value)]
> struct is_range_iterable : decltype(detail::is_range_iterable<T>(0)) {};
>                                     ^
> /redacted/git/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1194:30: note: in instantiation of template class 'llvm::is_range_iterable<const llvm::SmallVector<llvm::PointerIntPair<clang::Module *, 1, bool>, 2> &>' requested here
>       std::enable_if_t<llvm::is_range_iterable<Iterable>::value, bool> = true)
>                              ^
> /redacted/git/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1168:22: note: while substituting deduced template arguments into function template 'SmallVector' [with Iterable = const llvm::SmallVector<llvm::PointerIntPair<clang::Module *, 1, bool>, 2> &]
> class LLVM_GSL_OWNER SmallVector : public SmallVectorImpl<T>,
>                      ^
> /redacted/git/llvm-project/clang/include/clang/Basic/Module.h:96:7: note: while declaring the implicit copy constructor for 'Module'
> class Module {
>       ^
> /redacted/git/llvm-project/clang/include/clang/Basic/Module.h:96:7: note: definition of 'clang::Module' is not complete until the closing '}'
> 1 error generated.
> ```
For reference, the issue is that `std::next` takes its parameter by value: https://godbolt.org/z/531f3f86s.

(though I'm not sure we want to move forward with this patch given @Quuxplusone's comments)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102760