Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
DimitryAndric opened this issue Aug 28, 2017 · 5 comments
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema" crash-on-valid regression

Comments

@DimitryAndric
Copy link
Collaborator

DimitryAndric commented Aug 28, 2017

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:
0. 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

  1.  /tmp/vulkan-cpu/src/llvm_wrapper/llvm_wrapper.h:202:5: current parser token '{'
    
  2.  /tmp/vulkan-cpu/src/llvm_wrapper/llvm_wrapper.h:43:1: parsing namespace 'vulkan_cpu'
    
  3.  /tmp/vulkan-cpu/src/llvm_wrapper/llvm_wrapper.h:45:1: parsing namespace 'vulkan_cpu::llvm_wrapper'
    
  4.  /tmp/vulkan-cpu/src/llvm_wrapper/llvm_wrapper.h:189:1: parsing struct/union/class body 'vulkan_cpu::llvm_wrapper::Target'
    
  5.  /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
Copy link
Collaborator

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 (<typename... h>), now it contains two levels: (, <typename... h>). 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.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@shafik
Copy link
Collaborator

shafik commented May 9, 2023

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

@shafik shafik closed this as not planned Won't fix, can't repro, duplicate, stale May 9, 2023
@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed clang Clang issues not falling into any other category labels May 9, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented May 9, 2023

@llvm/issue-subscribers-clang-frontend

@DimitryAndric
Copy link
Collaborator Author

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

@Endilll
Copy link
Contributor

Endilll commented Jun 18, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema" crash-on-valid regression
Projects
None yet
Development

No branches or pull requests

6 participants