llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.11k stars 12.01k forks source link

After r305903, Assertion failed: (Replacement.isCanonical() && "replacement types must always be canonical"), function getSubstTemplateTypeParmType, #33691

Closed DimitryAndric closed 1 year ago

DimitryAndric commented 7 years ago
Bugzilla Link 34343
Version trunk
OS All
CC @emaste,@zygoloid,@spavloff

Extended Description

In https://bugs.freebsd.org/221864, Jan Beich describes how building a vulkan LLVM wrapper causes clang 5.0.0 rc2 to assert with: 'Assertion failed: (Replacement.isCanonical() && "replacement types must always be canonical"), function getSubstTemplateTypeParmType, file /poudriere/jails/head-amd64/usr/src/contrib/llvm/tools/clang/lib/AST/ASTContext.cpp, line 3520.'

This also reproduces on trunk r311836, resulting in:

Assertion failed: (Replacement.isCanonical() && "replacement types must always be canonical"), function getSubstTemplateTypeParmType, file /share/dim/src/llvm/trunk/tools/clang/lib/AST/ASTContext.cpp, line 3516.

​0 0x00000000012e2c08 llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/share/dim/llvm/311836-trunk-freebsd12-amd64-ninja-rel-1/bin/clang+0x12e2c08)

​1 0x00000000012e31f6 SignalHandler(int) (/share/dim/llvm/311836-trunk-freebsd12-amd64-ninja-rel-1/bin/clang+0x12e31f6)

​2 0x0000000803b0c8f6 handle_signal /usr/src/lib/libthr/thread/thr_sig.c:0:3

Stack dump:

  1. Program arguments: /share/dim/llvm/311836-trunk-freebsd12-amd64-ninja-rel-1/bin/clang -cc1 -triple x86_64-unknown-freebsd12.0 -emit-obj -mrelax-all -disable-free -main-file-name llvm_wrapper.cpp -mrelocation-model static -mthread-model posix -mdisable-fp-elim -masm-verbose -mconstructor-aliases -munwind-tables -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb -coverage-notes-file /tmp/vulkan-cpu/src/llvm_wrapper/CMakeFiles/vulkan_cpu_llvm_wrapper.dir/llvm_wrapper.cpp.gcno -D STDC_CONSTANT_MACROS -D STDC_FORMAT_MACROS -D __STDC_LIMIT_MACROS -Wall -Werror -Wno-error=#warnings -std=gnu++14 -fdeprecated-macro -ftemplate-depth 1024 -ferror-limit 19 -fmessage-length 101 -fobjc-runtime=gnustep -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -x c++ llvm_wrapper-eb0531.cpp
  2. /tmp/vulkan-cpu/src/llvm_wrapper/llvm_wrapper.h:202:5: current parser token '{'
  3. /tmp/vulkan-cpu/src/llvm_wrapper/llvm_wrapper.h:43:1: parsing namespace 'vulkan_cpu'
  4. /tmp/vulkan-cpu/src/llvm_wrapper/llvm_wrapper.h:45:1: parsing namespace 'vulkan_cpu::llvm_wrapper'
  5. /tmp/vulkan-cpu/src/llvm_wrapper/llvm_wrapper.h:189:1: parsing struct/union/class body 'vulkan_cpu::llvm_wrapper::Target'
  6. /tmp/vulkan-cpu/src/util/variant.h:895:7: instantiating class definition 'vulkan_cpu::util::variant<vulkan_cpu::llvm_wrapper::Target, vulkan_cpu::llvm_wrapper::LLVM_string>'

Bisection shows that this started occurring after https://reviews.llvm.org/rL305903 ("Function with unparsed body is a definition"), which is a fix for bug 14785.

Minimized test case:

// clang -cc1 -triple x86_64 -S -std=c++11 -fcxx-exceptions -fexceptions testcase.cpp

template <int, class> struct a;
namespace b {
constexpr int c() { return 0; }
template <int, typename> struct d;
template <typename... e> using f = d<0, e...>;
}
template <typename> struct g {
  template <typename... h>
  friend constexpr typename a<b::f<h...>::i, bool>::j
  operator==(const g<h...> &, const g<h...> &) noexcept(b::f<h...>::k);
};
template <typename... e>
constexpr typename a<b::f<e...>::i, bool>::j
operator==(const g<e...> &, const g<e...> &) noexcept(b::f<e...>::k);
void l(g<int>) {}
spavloff commented 7 years ago

Revision 305903 introduced the change into SemaTemplateInstantiateDecl.cpp:

if (isFriend) Function->setObjectOfFriendDecl();

Previously an instantiation of a friend function was not marked as a friend declaration, which is wrong. As a result the code in Sema::getTemplateInstantiationArgs is executed:

  // If this is a friend declaration and it declares an entity at
  // namespace scope, take arguments from its lexical parent
  // instead of its semantic parent, unless of course the pattern we're
  // instantiating actually comes from the file's context!
  if (Function->getFriendObjectKind() &&
      Function->getDeclContext()->isFileContext() &&
      (!Pattern || !Pattern->getLexicalDeclContext()->isFileContext())) {
    Ctx = Function->getLexicalDeclContext();
    RelativeToPrimary = false;
    continue;
  }

It changed instantiation stack for the friend operator. Previously it contained only (), now it contains two levels: (, ). Both are wrong as we do not have arguments for <typename… h>, it is a template parameter. The correct state of state of instantiation stack would be (), a template argument of containing class. Previously code worked due to behavior of Sema::CheckParameterPacksForExpansion:

  // If we don't have a template argument at this depth/index, then we 
  // cannot expand the pack expansion. Make a note of this, but we still 
  // want to check any parameter packs we *do* have arguments for.
  if (Depth >= TemplateArgs.getNumLevels() ||
      !TemplateArgs.hasTemplateArgument(Depth, Index)) {
    ShouldExpand = false;
    continue;
  }

Depth of the template function parameter <typename… h> was 0. It is incorrect but template argument stack was incorrect too (<typename… h>) and ShouldExpand was set to false, it prevented the pack expansion. Now 'Depth' has right value (1), but template argument stack is wrong (, <typename… h>). ShouldExpand is set to true, but instantiation stack contains template parameters instead of template arguments and assertion check is fired.

Instantiation of friend function templates was fixed in the patch https://reviews.llvm.org/D21767. With it the code presented in this report compiles successfully.

shafik commented 1 year ago

No longer reproduces on trunk: https://godbolt.org/z/s6KjceTh1

llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-frontend

DimitryAndric commented 1 year ago

FWIW this was fixed in af10d6f350ff3e92c6ffae66cc9dac36884cdd55 ("[clang] don't instantiate templates with injected arguments") by @mizvekov, back in 2021. (clang >= 14)

Endilll commented 1 year ago

Fixed in Clang 14 indeed: https://godbolt.org/z/j8foGh7W8