[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D91488#2451755 , @teemperor wrote:

> I believe rsmith is in GMT-8, so I assume this won't get a fix soon and I 
> went ahead and reverted in 22ccdb787024e954318e35fcf904fd4fa36f5679 
> 



In D91488#2451760 , @davezarzycki 
wrote:

> Thanks. I just verified that reverting this change fixes the second stage 
> regression and was about to commit the revert.

Thanks for the revert and verification. I saw these failures yesterday, but I 
incorrectly thought I saw the same failures in stage1, so they couldn't 
possibly be due to my change because they were compilation failures in LLVM. 
Oops!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

Thanks. I just verified that reverting this change fixes the second stage 
regression and was about to commit the revert.

@rsmith – If you need help testing a fix, please let me know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

I believe rsmith is in GMT-8, so I assume this won't get a fix soon and I went 
ahead and reverted in 22ccdb787024e954318e35fcf904fd4fa36f5679 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

This change is causing second stage build failures on Fedora 33 (x86-64). I'll 
probably revert this soon, but in the mean time, here is a snippet of the build 
output:

  FAILED: 
lib/ExecutionEngine/JITLink/CMakeFiles/LLVMJITLink.dir/JITLinkGeneric.cpp.o
  /p/tllvm/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Ilib/ExecutionEngine/JITLink 
-I/home/dave/ro_s/lp/llvm/lib/ExecutionEngine/JITLink -Iinclude 
-I/home/dave/ro_s/lp/llvm/include -Werror=switch -Wno-deprecated-copy 
-stdlib=libc++ -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 -Wimplicit-fallthrough -Wcovered-switch-default 
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor 
-Wsuggest-override -Wstring-conversion -fdiagnostics-color -ffunction-sections 
-fdata-sections -O2   -march=skylake -fno-vectorize -fno-slp-vectorize 
-fno-tree-slp-vectorize -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT 
lib/ExecutionEngine/JITLink/CMakeFiles/LLVMJITLink.dir/JITLinkGeneric.cpp.o -MF 
lib/ExecutionEngine/JITLink/CMakeFiles/LLVMJITLink.dir/JITLinkGeneric.cpp.o.d 
-o lib/ExecutionEngine/JITLink/CMakeFiles/LLVMJITLink.dir/JITLinkGeneric.cpp.o 
-c /home/dave/ro_s/lp/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
  In file included from 
/home/dave/ro_s/lp/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp:13:
  /home/dave/ro_s/lp/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h:150:18: 
error: invalid operands to binary expression 
('llvm::jitlink::LinkGraph::nested_collection_iterator *>, llvm::jitlink::Section>, 
llvm::detail::DenseSetImpl, 
llvm::detail::DenseSetPair>, 
llvm::DenseMapInfo>::Iterator, llvm::jitlink::Block *, 
::jitlink::LinkGraph::getSectionBlocks>' and 
'llvm::jitlink::LinkGraph::nested_collection_iterator *>, llvm::jitlink::Section>, 
llvm::detail::DenseSetImpl, 
llvm::detail::DenseSetPair>, 
llvm::DenseMapInfo>::Iterator, llvm::jitlink::Block *, 
::jitlink::LinkGraph::getSectionBlocks>')
  for (auto *B : G.blocks()) {
   ^
  /home/dave/ro_s/lp/llvm/include/llvm/ADT/APInt.h:2037:13: note: candidate 
function not viable: no known conversion from 
'llvm::jitlink::LinkGraph::nested_collection_iterator *>, llvm::jitlink::Section>, 
llvm::detail::DenseSetImpl, 
llvm::detail::DenseSetPair>, 
llvm::DenseMapInfo>::Iterator, llvm::jitlink::Block *, 
::jitlink::LinkGraph::getSectionBlocks>' to 'uint64_t' (aka 'unsigned 
long') for 1st argument
  inline bool operator!=(uint64_t V1, const APInt ) { return V2 != V1; }
  ^
  /home/dave/ro_s/lp/llvm/include/llvm/ADT/APSInt.h:340:13: note: candidate 
function not viable: no known conversion from 
'llvm::jitlink::LinkGraph::nested_collection_iterator *>, llvm::jitlink::Section>, 
llvm::detail::DenseSetImpl, 
llvm::detail::DenseSetPair>, 
llvm::DenseMapInfo>::Iterator, llvm::jitlink::Block *, 
::jitlink::LinkGraph::getSectionBlocks>' to 'int64_t' (aka 'long') for 1st 
argument
  inline bool operator!=(int64_t V1, const APSInt ) { return V2 != V1; }
  ^
  /home/dave/ro_s/lp/llvm/include/llvm/ADT/StringRef.h:904:15: note: candidate 
function not viable: no known conversion from 
'llvm::jitlink::LinkGraph::nested_collection_iterator *>, llvm::jitlink::Section>, 
llvm::detail::DenseSetImpl, 
llvm::detail::DenseSetPair>, 
llvm::DenseMapInfo>::Iterator, llvm::jitlink::Block *, 
::jitlink::LinkGraph::getSectionBlocks>' to 'llvm::StringRef' for 1st 
argument
inline bool operator!=(StringRef LHS, StringRef RHS) { return !(LHS == 
RHS); }
^
  /p/tllvm/bin/../include/c++/v1/system_error:419:1: note: candidate function 
not viable: no known conversion from 
'llvm::jitlink::LinkGraph::nested_collection_iterator *>, llvm::jitlink::Section>, 
llvm::detail::DenseSetImpl, 
llvm::detail::DenseSetPair>, 
llvm::DenseMapInfo>::Iterator, llvm::jitlink::Block *, 
::jitlink::LinkGraph::getSectionBlocks>' to 'const std::error_code' for 
1st argument
  operator!=(const error_code& __x, const error_code& __y) _NOEXCEPT
  ^
  /p/tllvm/bin/../include/c++/v1/system_error:424:1: note: candidate function 
not viable: no known conversion from 
'llvm::jitlink::LinkGraph::nested_collection_iterator *>, llvm::jitlink::Section>, 
llvm::detail::DenseSetImpl, 
llvm::detail::DenseSetPair>, 
llvm::DenseMapInfo>::Iterator, llvm::jitlink::Block *, 
::jitlink::LinkGraph::getSectionBlocks>' to 'const std::error_code' for 
1st argument
  operator!=(const error_code& __x, const error_condition& __y) _NOEXCEPT
  ^
  /p/tllvm/bin/../include/c++/v1/system_error:429:1: note: candidate function 
not viable: no known conversion from 
'llvm::jitlink::LinkGraph::nested_collection_iterator *>, llvm::jitlink::Section>, 
llvm::detail::DenseSetImpl, 
llvm::detail::DenseSetPair>, 

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

This also crashes test-suite, so I've reverted the change for now. (Stack trace 
for tramp3d-v4 crash: 
https://llvm-compile-time-tracker.com/show_error.php?commit=7b3470baf8bab1919e3ad4c18e2b776c1f7be2d5)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

Clang is crashing after this change when bootstrapping and building 
lib/Support/CMakeFiles/LLVMSupport.dir/Hashing.cpp.o
(our log if it helps: 
https://buildkite.com/mlir/mlir-core/builds/10003#80a5b6a9-d8d9-4420-ab2a-c24f398bc5d4
 )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b3470baf8ba: Consider reference, pointer, and 
pointer-to-member TemplateArguments to be… (authored by rsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/clang-abi-compat.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -187,6 +187,11 @@
 int  = f(B<>());
 float  = f(B<>());
 
+void type_affects_identity(B<>) {}
+void type_affects_identity(B<(const int*)>) {}
+void type_affects_identity(B<(void*)>) {}
+void type_affects_identity(B<(const void*)>) {}
+
 // pointers to members
 template struct B {};
 template struct B {};
@@ -198,6 +203,12 @@
 char  = f(B<::p>());
 short  = f(B<::pp>());
 
+struct Y : X {};
+void type_affects_identity(B<::n>) {}
+void type_affects_identity(B<(int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+void type_affects_identity(B<(const int X::*)::n>) {}
+void type_affects_identity(B<(const int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+
 // A case where we need to do auto-deduction, and check whether the
 // resulting dependent types match during partial ordering. These
 // templates are not ordered due to the mismatching function parameter.
Index: clang/test/CodeGenCXX/mangle-template.cpp
===
--- clang/test/CodeGenCXX/mangle-template.cpp
+++ clang/test/CodeGenCXX/mangle-template.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++20 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s --check-prefixes=CHECK,CXX20
 // expected-no-diagnostics
 
 namespace test1 {
@@ -221,3 +222,78 @@
   void g() { f(1, 2); }
 }
 
+#if __cplusplus >= 202002L
+namespace cxx20 {
+  template struct A {};
+  template struct B {};
+
+  int x;
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1xE(
+  void f(A<>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKiadL_ZNS_1xE(
+  void f(A<(const int*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPvadL_ZNS_1xE(
+  void f(A<(void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPvXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKvadL_ZNS_1xE(
+  void f(A<(const void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKvXadL_ZNS_1xE(
+  void f(B) {}
+
+  struct Q { int x; };
+
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1Q1xE(
+  void f(A<::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEiXadL_ZNS1_1xE
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvMNS_1QEKiadL_ZNS1_1xE(
+  void f(A<(const int Q::*)::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEKiXadL_ZNS1_1xE(
+  void f(B) {}
+}
+#endif
+
+namespace test17 {
+  // Ensure we mangle the types for non-type template arguments if we've lost
+  // track of argument / parameter correspondence.
+  template struct X {};
+
+  // CHECK: define {{.*}} @_ZN6test171fILi1EJLi2ELi3ELi4vNS_1XIXT_EJLi5EXspT0_ELi6
+  template void f(X) {}
+  void g() { f<1, 2, 3, 4>({}); }
+
+  // Note: there is no J...E here, because we can't form a pack argument, and
+  // the 5u and 6u are mangled with the original type 'j' (unsigned int) not
+  // with the resolved type 'i' (signed int).
+  // CHECK: define {{.*}} @_ZN6test171hILi4EJLi1ELi2ELi3vNS_1XIXspT0_EXLj5EEXT_EXLj6
+  template void h(X) {}
+  void i() { h<4, 1, 2, 3>({}); }
+
+#if __cplusplus >= 201402L
+  template struct Y {};
+  int n;
+  // Case 1:  is a resolved template argument, with a known parameter:
+  // mangled with no conversion.
+  // CXX20: define {{.*}} @_ZN6test172j1ILi1EEEvNS_1YIXT_EXadL_ZNS_1nE
+  template void j1(Y) {}
+  // Case 2:  is an unresolved template argument, with an unknown
+  // corresopnding parameter: mangled as the source expression.
+  // CXX20: define {{.*}} @_ZN6test172j2IJLi1vNS_1YIXspT_EXcvPKiadL_ZNS_1nE
+  template void j2(Y) {}
+  // Case 3:  is a resolved 

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4847
+  /// Do we need to mangle template arguments with exactly correct types?
+  bool needExactType(unsigned I) const {
+// We need correct types when the template-name is unresolved or when it

rjmccall wrote:
> rsmith wrote:
> > rjmccall wrote:
> > > This comment should probably say explicitly that I is the parameter 
> > > index.  Do we need to worry about packs that haven't yet been packed?  Is 
> > > it true that if we have a resolved template then we should always have 
> > > packed the matching arguments?
> > Parameter renamed to `ParamIdx`.
> > 
> > Regarding packs, yes, that was my expectation (and it holds for all current 
> > examples in Clang's test suite), but it doesn't appear to be true in full 
> > generality. Here's a counterexample:
> > 
> > ```
> > template struct X {};
> > 
> > // Everyone agrees that the template arguments here are I (D) J (C...) E E
> > template void h(X) {}
> > void i() { h<1, 2, 3, 4>(X<1, 2, 3, 4>()); }
> > 
> > // But everyone agrees that the template arguments here are I (C...) (D) E, 
> > with no J...E in sight
> > template void f(X) {}
> > void g() { f<4, 1, 2, 3>(X<1, 2, 3, 4>()); }
> > ```
> > 
> > I think the above example demonstrates a pre-existing hole in the ABI rule, 
> > which I would imagine we fix by following the implementations: if a 
> > left-to-right traversal of the parameters and arguments results in a pack 
> > expansion corresponding to a non-pack, then from that point onwards we 
> > don't form any `J...E` manglings and template arguments in an unresolved 
> > form (in particular, mangle non-type arguments as expressions).
> > 
> > I've added a test for that case. We don't strictly need to do anything 
> > special here to handle that, because it doesn't matter what `needExactType` 
> > returns when mangling an unresolved template argument, but it seems better 
> > to more precisely track whether we expect to see matching arguments and 
> > parameters or not, so we now track that here too.
> > 
> > Thanks for catching this!
> Ugh.  I think the right rule in the abstract would be to always mangle 
> template arguments in a dependent template argument list using their written 
> structure, even when we can immediately resolve the target template and check 
> arguments against parameters.  That would certainly include not trying to 
> collect arguments into template parameter packs.  But that ship probably 
> sailed a long time ago, and your left-to-right rule is presumably what we're 
> left with.  We should document that in the ABI, though; mind opening a PR?
Not sure if PR in this context is problem report or pull request, but: 
https://github.com/itanium-cxx-abi/cxx-abi/issues/113


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Seems fine to me.




Comment at: clang/lib/AST/ItaniumMangle.cpp:4847
+  /// Do we need to mangle template arguments with exactly correct types?
+  bool needExactType(unsigned I) const {
+// We need correct types when the template-name is unresolved or when it

rsmith wrote:
> rjmccall wrote:
> > This comment should probably say explicitly that I is the parameter index.  
> > Do we need to worry about packs that haven't yet been packed?  Is it true 
> > that if we have a resolved template then we should always have packed the 
> > matching arguments?
> Parameter renamed to `ParamIdx`.
> 
> Regarding packs, yes, that was my expectation (and it holds for all current 
> examples in Clang's test suite), but it doesn't appear to be true in full 
> generality. Here's a counterexample:
> 
> ```
> template struct X {};
> 
> // Everyone agrees that the template arguments here are I (D) J (C...) E E
> template void h(X) {}
> void i() { h<1, 2, 3, 4>(X<1, 2, 3, 4>()); }
> 
> // But everyone agrees that the template arguments here are I (C...) (D) E, 
> with no J...E in sight
> template void f(X) {}
> void g() { f<4, 1, 2, 3>(X<1, 2, 3, 4>()); }
> ```
> 
> I think the above example demonstrates a pre-existing hole in the ABI rule, 
> which I would imagine we fix by following the implementations: if a 
> left-to-right traversal of the parameters and arguments results in a pack 
> expansion corresponding to a non-pack, then from that point onwards we don't 
> form any `J...E` manglings and template arguments in an unresolved form (in 
> particular, mangle non-type arguments as expressions).
> 
> I've added a test for that case. We don't strictly need to do anything 
> special here to handle that, because it doesn't matter what `needExactType` 
> returns when mangling an unresolved template argument, but it seems better to 
> more precisely track whether we expect to see matching arguments and 
> parameters or not, so we now track that here too.
> 
> Thanks for catching this!
Ugh.  I think the right rule in the abstract would be to always mangle template 
arguments in a dependent template argument list using their written structure, 
even when we can immediately resolve the target template and check arguments 
against parameters.  That would certainly include not trying to collect 
arguments into template parameter packs.  But that ship probably sailed a long 
time ago, and your left-to-right rule is presumably what we're left with.  We 
should document that in the ABI, though; mind opening a PR?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith marked an inline comment as done.
rsmith added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3758
   TemplateArgument TA(T->getNumBitsExpr());
-  mangleTemplateArgs(, 1);
+  mangleTemplateArgs(TemplateName(), , 1);
   if (T->isUnsigned())

rjmccall wrote:
> Passing a null TemplateName here causes these to get the unresolved 
> treatment, which means mangling the exact type, right?  I guess it just never 
> matters for integer arguments?
Yes, it never matters for expressions (this function), nor for evaluated 
integer constants (the previous function).



Comment at: clang/lib/AST/ItaniumMangle.cpp:4847
+  /// Do we need to mangle template arguments with exactly correct types?
+  bool needExactType(unsigned I) const {
+// We need correct types when the template-name is unresolved or when it

rjmccall wrote:
> This comment should probably say explicitly that I is the parameter index.  
> Do we need to worry about packs that haven't yet been packed?  Is it true 
> that if we have a resolved template then we should always have packed the 
> matching arguments?
Parameter renamed to `ParamIdx`.

Regarding packs, yes, that was my expectation (and it holds for all current 
examples in Clang's test suite), but it doesn't appear to be true in full 
generality. Here's a counterexample:

```
template struct X {};

// Everyone agrees that the template arguments here are I (D) J (C...) E E
template void h(X) {}
void i() { h<1, 2, 3, 4>(X<1, 2, 3, 4>()); }

// But everyone agrees that the template arguments here are I (C...) (D) E, 
with no J...E in sight
template void f(X) {}
void g() { f<4, 1, 2, 3>(X<1, 2, 3, 4>()); }
```

I think the above example demonstrates a pre-existing hole in the ABI rule, 
which I would imagine we fix by following the implementations: if a 
left-to-right traversal of the parameters and arguments results in a pack 
expansion corresponding to a non-pack, then from that point onwards we don't 
form any `J...E` manglings and template arguments in an unresolved form (in 
particular, mangle non-type arguments as expressions).

I've added a test for that case. We don't strictly need to do anything special 
here to handle that, because it doesn't matter what `needExactType` returns 
when mangling an unresolved template argument, but it seems better to more 
precisely track whether we expect to see matching arguments and parameters or 
not, so we now track that here too.

Thanks for catching this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 311008.
rsmith added a comment.

- Handle @rjmccall's review feedback.
- Properly handle the case of a pack expansion into a non-pack and add tests 
for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/clang-abi-compat.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -187,6 +187,11 @@
 int  = f(B<>());
 float  = f(B<>());
 
+void type_affects_identity(B<>) {}
+void type_affects_identity(B<(const int*)>) {}
+void type_affects_identity(B<(void*)>) {}
+void type_affects_identity(B<(const void*)>) {}
+
 // pointers to members
 template struct B {};
 template struct B {};
@@ -198,6 +203,12 @@
 char  = f(B<::p>());
 short  = f(B<::pp>());
 
+struct Y : X {};
+void type_affects_identity(B<::n>) {}
+void type_affects_identity(B<(int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+void type_affects_identity(B<(const int X::*)::n>) {}
+void type_affects_identity(B<(const int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+
 // A case where we need to do auto-deduction, and check whether the
 // resulting dependent types match during partial ordering. These
 // templates are not ordered due to the mismatching function parameter.
Index: clang/test/CodeGenCXX/mangle-template.cpp
===
--- clang/test/CodeGenCXX/mangle-template.cpp
+++ clang/test/CodeGenCXX/mangle-template.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++20 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s --check-prefixes=CHECK,CXX20
 // expected-no-diagnostics
 
 namespace test1 {
@@ -221,3 +222,78 @@
   void g() { f(1, 2); }
 }
 
+#if __cplusplus >= 202002L
+namespace cxx20 {
+  template struct A {};
+  template struct B {};
+
+  int x;
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1xE(
+  void f(A<>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKiadL_ZNS_1xE(
+  void f(A<(const int*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPvadL_ZNS_1xE(
+  void f(A<(void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPvXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKvadL_ZNS_1xE(
+  void f(A<(const void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKvXadL_ZNS_1xE(
+  void f(B) {}
+
+  struct Q { int x; };
+
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1Q1xE(
+  void f(A<::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEiXadL_ZNS1_1xE
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvMNS_1QEKiadL_ZNS1_1xE(
+  void f(A<(const int Q::*)::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEKiXadL_ZNS1_1xE(
+  void f(B) {}
+}
+#endif
+
+namespace test17 {
+  // Ensure we mangle the types for non-type template arguments if we've lost
+  // track of argument / parameter correspondence.
+  template struct X {};
+
+  // CHECK: define {{.*}} @_ZN6test171fILi1EJLi2ELi3ELi4vNS_1XIXT_EJLi5EXspT0_ELi6
+  template void f(X) {}
+  void g() { f<1, 2, 3, 4>({}); }
+
+  // Note: there is no J...E here, because we can't form a pack argument, and
+  // the 5u and 6u are mangled with the original type 'j' (unsigned int) not
+  // with the resolved type 'i' (signed int).
+  // CHECK: define {{.*}} @_ZN6test171hILi4EJLi1ELi2ELi3vNS_1XIXspT0_EXLj5EEXT_EXLj6
+  template void h(X) {}
+  void i() { h<4, 1, 2, 3>({}); }
+
+#if __cplusplus >= 201402L
+  template struct Y {};
+  int n;
+  // Case 1:  is a resolved template argument, with a known parameter:
+  // mangled with no conversion.
+  // CXX20: define {{.*}} @_ZN6test172j1ILi1EEEvNS_1YIXT_EXadL_ZNS_1nE
+  template void j1(Y) {}
+  // Case 2:  is an unresolved template argument, with an unknown
+  // corresopnding parameter: mangled as the source expression.
+  // CXX20: define {{.*}} @_ZN6test172j2IJLi1vNS_1YIXspT_EXcvPKiadL_ZNS_1nE
+  template void j2(Y) {}
+  // Case 3:  is a resolved template argument, with a known parameter, but
+  // for a 

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3758
   TemplateArgument TA(T->getNumBitsExpr());
-  mangleTemplateArgs(, 1);
+  mangleTemplateArgs(TemplateName(), , 1);
   if (T->isUnsigned())

Passing a null TemplateName here causes these to get the unresolved treatment, 
which means mangling the exact type, right?  I guess it just never matters for 
integer arguments?



Comment at: clang/lib/AST/ItaniumMangle.cpp:4847
+  /// Do we need to mangle template arguments with exactly correct types?
+  bool needExactType(unsigned I) const {
+// We need correct types when the template-name is unresolved or when it

This comment should probably say explicitly that I is the parameter index.  Do 
we need to worry about packs that haven't yet been packed?  Is it true that if 
we have a resolved template then we should always have packed the matching 
arguments?



Comment at: clang/lib/AST/ItaniumMangle.cpp:4963
+if (D->isCXXInstanceMember())
+  Value = APValue(D, false, {});
+else if (D->getType()->isArrayType() &&

Could you `/*comment*/` the false here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 310741.
rsmith added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/clang-abi-compat.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -187,6 +187,11 @@
 int  = f(B<>());
 float  = f(B<>());
 
+void type_affects_identity(B<>) {}
+void type_affects_identity(B<(const int*)>) {}
+void type_affects_identity(B<(void*)>) {}
+void type_affects_identity(B<(const void*)>) {}
+
 // pointers to members
 template struct B {};
 template struct B {};
@@ -198,6 +203,12 @@
 char  = f(B<::p>());
 short  = f(B<::pp>());
 
+struct Y : X {};
+void type_affects_identity(B<::n>) {}
+void type_affects_identity(B<(int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+void type_affects_identity(B<(const int X::*)::n>) {}
+void type_affects_identity(B<(const int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+
 // A case where we need to do auto-deduction, and check whether the
 // resulting dependent types match during partial ordering. These
 // templates are not ordered due to the mismatching function parameter.
Index: clang/test/CodeGenCXX/mangle-template.cpp
===
--- clang/test/CodeGenCXX/mangle-template.cpp
+++ clang/test/CodeGenCXX/mangle-template.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++20 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s --check-prefixes=CHECK,CXX20
 // expected-no-diagnostics
 
 namespace test1 {
@@ -221,3 +222,38 @@
   void g() { f(1, 2); }
 }
 
+#if __cplusplus >= 202002L
+namespace cxx20 {
+  template struct A {};
+  template struct B {};
+
+  int x;
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1xE(
+  void f(A<>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKiadL_ZNS_1xE(
+  void f(A<(const int*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPvadL_ZNS_1xE(
+  void f(A<(void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPvXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKvadL_ZNS_1xE(
+  void f(A<(const void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKvXadL_ZNS_1xE(
+  void f(B) {}
+
+  struct Q { int x; };
+
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1Q1xE(
+  void f(A<::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEiXadL_ZNS1_1xE
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvMNS_1QEKiadL_ZNS1_1xE(
+  void f(A<(const int Q::*)::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEKiXadL_ZNS1_1xE(
+  void f(B) {}
+}
+#endif
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -124,7 +124,7 @@
 template void f() {}
 
 // Union members.
-// CHECK: define weak_odr void @_Z1fIXL1vv(
+// CHECK: define weak_odr void @_Z1fIL1EEEvv(
 // MSABI: define {{.*}} @"??$f@$7TE@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1vv(
@@ -214,10 +214,10 @@
 template void f() {}
 template void f() {}
 template void f() {}
-// CHECK: define weak_odr void @_Z1fIXL2H1EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H1EEvv
 // MSABI: define {{.*}} @"??$f@$7TH1@YAXXZ"
 template void f();
-// CHECK: define weak_odr void @_Z1fIXL2H2EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H2EEvv
 // MSABI: define {{.*}} @"??$f@$7TH2@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2H3EEEvv
Index: clang/test/CodeGenCXX/clang-abi-compat.cpp
===
--- clang/test/CodeGenCXX/clang-abi-compat.cpp
+++ clang/test/CodeGenCXX/clang-abi-compat.cpp
@@ -1,10 +1,12 @@
-// RUN: %clang_cc1 -std=c++17 -triple x86_64-linux-gnu -fclang-abi-compat=3.0 %s -emit-llvm -o - | FileCheck --check-prefixes=PRE39,PRE5,PRE12 %s
-// RUN: %clang_cc1 -std=c++17 -triple x86_64-linux-gnu -fclang-abi-compat=3.8 %s 

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4861
+// We need correct types when the template-name is unresolved or when it
+// might be overloaded.
+if (!ResolvedTemplate)

Quuxplusone wrote:
> And from the PR summary:
> 
> > namely mangling such template arguments as being cast to the parameter type 
> > in the case where the template name is unresolved or overloaded
> 
> This phrasing worries me a little bit. Are you saying that you might mangle 
> the name of `foo` in one way, when there's also a `foo Args>` in scope, and in a different way, when there's not? That doesn't seem 
> conforming. So I imagine it's more likely that I'm misunderstanding what you 
> mean by "might be overloaded" / "is overloaded". Could you explain for my 
> information, and perhaps also adjust the wording of these code comments to 
> make the explanation less needed?
> 
> Specifically, I think it would be non-conforming if the TU
> 
> // https://godbolt.org/z/YjPqMd
> template void foo();
> char arr[6];
> extern template void foo();  // #1
> int main() { foo(); }
> 
> could not be linked against the TU
> 
> template int foo();
> template void foo();  // is this "overloading"?
> extern char arr[6];
> template<> void foo() {}  // #2
> 
> because lines #1 and #2 disagreed about the way to mangle `foo`. (Again, 
> I'm pretty sure you haven't made them disagree... but I remain unclear on 
> what's meant by "overloading" in this PR, if it's //not// this.)
Well, technically, a single function template is considered overloaded all by 
itself :) ... but no, this is a typo in the change description. I meant 
"overloadable", not "overloaded". The proposed Itanium ABI rule applies to 
function templates (other than the call operator or conversion function of a 
generic lambda).

The "might be overloaded" here means "might be overloaded by some other 
(earlier or later or in a different TU) declaration", so I don't think that's 
wrong, but I'll rephrase it for the avoidance of any doubt.

(In principle the ABI rule also applies to cases where the template-name is 
unresolved, but it only makes a difference if the type of the template 
parameter is known and the template argument is not (eg, if it's the argument 
of a template template parameter). I suspect that's actually impossible, 
because the expression or type would need to be instantiation-dependent in that 
case, so we'd mangle the original syntax for the template argument not the 
result of converting it to the parameter type.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 305393.
rsmith added a comment.

Tweak wording of comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/clang-abi-compat.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -187,6 +187,11 @@
 int  = f(B<>());
 float  = f(B<>());
 
+void type_affects_identity(B<>) {}
+void type_affects_identity(B<(const int*)>) {}
+void type_affects_identity(B<(void*)>) {}
+void type_affects_identity(B<(const void*)>) {}
+
 // pointers to members
 template struct B {};
 template struct B {};
@@ -198,6 +203,12 @@
 char  = f(B<::p>());
 short  = f(B<::pp>());
 
+struct Y : X {};
+void type_affects_identity(B<::n>) {}
+void type_affects_identity(B<(int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+void type_affects_identity(B<(const int X::*)::n>) {}
+void type_affects_identity(B<(const int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+
 // A case where we need to do auto-deduction, and check whether the
 // resulting dependent types match during partial ordering. These
 // templates are not ordered due to the mismatching function parameter.
Index: clang/test/CodeGenCXX/mangle-template.cpp
===
--- clang/test/CodeGenCXX/mangle-template.cpp
+++ clang/test/CodeGenCXX/mangle-template.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++20 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s --check-prefixes=CHECK,CXX20
 // expected-no-diagnostics
 
 namespace test1 {
@@ -212,3 +213,39 @@
 template __make_integer_seq make<5>();
 // CHECK: define weak_odr {{.*}} @_ZN6test154makeILi5EEE18__make_integer_seqISt16integer_sequenceiXT_EEv(
 }
+
+#if __cplusplus >= 202002L
+namespace cxx20 {
+  template struct A {};
+  template struct B {};
+
+  int x;
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1xE(
+  void f(A<>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKiadL_ZNS_1xE(
+  void f(A<(const int*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPvadL_ZNS_1xE(
+  void f(A<(void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPvXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKvadL_ZNS_1xE(
+  void f(A<(const void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKvXadL_ZNS_1xE(
+  void f(B) {}
+
+  struct Q { int x; };
+
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1Q1xE(
+  void f(A<::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEiXadL_ZNS1_1xE
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvMNS_1QEKiadL_ZNS1_1xE(
+  void f(A<(const int Q::*)::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEKiXadL_ZNS1_1xE(
+  void f(B) {}
+}
+#endif
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -117,7 +117,7 @@
 template void f() {}
 
 // Union members.
-// CHECK: define weak_odr void @_Z1fIXL1vv(
+// CHECK: define weak_odr void @_Z1fIL1EEEvv(
 // FIXME: MSVC rejects this; check this is the mangling MSVC uses when they
 // start accepting.
 // MSABI: define {{.*}} @"??$f@$2TE@YAXXZ"
@@ -209,10 +209,10 @@
 template void f() {}
 template void f() {}
 template void f() {}
-// CHECK: define weak_odr void @_Z1fIXL2H1EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H1EEvv
 // MSABI: define {{.*}} @"??$f@$2TH1@YAXXZ"
 template void f();
-// CHECK: define weak_odr void @_Z1fIXL2H2EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H2EEvv
 // MSABI: define {{.*}} @"??$f@$2TH2@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2H3EEEvv
Index: clang/test/CodeGenCXX/clang-abi-compat.cpp
===
--- clang/test/CodeGenCXX/clang-abi-compat.cpp
+++ clang/test/CodeGenCXX/clang-abi-compat.cpp
@@ -1,9 +1,12 @@
-// RUN: %clang_cc1 -std=c++17 -triple x86_64-linux-gnu 

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 305392.
rsmith added a comment.

Improve commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/clang-abi-compat.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -187,6 +187,11 @@
 int  = f(B<>());
 float  = f(B<>());
 
+void type_affects_identity(B<>) {}
+void type_affects_identity(B<(const int*)>) {}
+void type_affects_identity(B<(void*)>) {}
+void type_affects_identity(B<(const void*)>) {}
+
 // pointers to members
 template struct B {};
 template struct B {};
@@ -198,6 +203,12 @@
 char  = f(B<::p>());
 short  = f(B<::pp>());
 
+struct Y : X {};
+void type_affects_identity(B<::n>) {}
+void type_affects_identity(B<(int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+void type_affects_identity(B<(const int X::*)::n>) {}
+void type_affects_identity(B<(const int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+
 // A case where we need to do auto-deduction, and check whether the
 // resulting dependent types match during partial ordering. These
 // templates are not ordered due to the mismatching function parameter.
Index: clang/test/CodeGenCXX/mangle-template.cpp
===
--- clang/test/CodeGenCXX/mangle-template.cpp
+++ clang/test/CodeGenCXX/mangle-template.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++20 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s --check-prefixes=CHECK,CXX20
 // expected-no-diagnostics
 
 namespace test1 {
@@ -212,3 +213,39 @@
 template __make_integer_seq make<5>();
 // CHECK: define weak_odr {{.*}} @_ZN6test154makeILi5EEE18__make_integer_seqISt16integer_sequenceiXT_EEv(
 }
+
+#if __cplusplus >= 202002L
+namespace cxx20 {
+  template struct A {};
+  template struct B {};
+
+  int x;
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1xE(
+  void f(A<>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKiadL_ZNS_1xE(
+  void f(A<(const int*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPvadL_ZNS_1xE(
+  void f(A<(void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPvXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKvadL_ZNS_1xE(
+  void f(A<(const void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKvXadL_ZNS_1xE(
+  void f(B) {}
+
+  struct Q { int x; };
+
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1Q1xE(
+  void f(A<::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEiXadL_ZNS1_1xE
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvMNS_1QEKiadL_ZNS1_1xE(
+  void f(A<(const int Q::*)::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEKiXadL_ZNS1_1xE(
+  void f(B) {}
+}
+#endif
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -117,7 +117,7 @@
 template void f() {}
 
 // Union members.
-// CHECK: define weak_odr void @_Z1fIXL1vv(
+// CHECK: define weak_odr void @_Z1fIL1EEEvv(
 // FIXME: MSVC rejects this; check this is the mangling MSVC uses when they
 // start accepting.
 // MSABI: define {{.*}} @"??$f@$2TE@YAXXZ"
@@ -209,10 +209,10 @@
 template void f() {}
 template void f() {}
 template void f() {}
-// CHECK: define weak_odr void @_Z1fIXL2H1EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H1EEvv
 // MSABI: define {{.*}} @"??$f@$2TH1@YAXXZ"
 template void f();
-// CHECK: define weak_odr void @_Z1fIXL2H2EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H2EEvv
 // MSABI: define {{.*}} @"??$f@$2TH2@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2H3EEEvv
Index: clang/test/CodeGenCXX/clang-abi-compat.cpp
===
--- clang/test/CodeGenCXX/clang-abi-compat.cpp
+++ clang/test/CodeGenCXX/clang-abi-compat.cpp
@@ -1,9 +1,12 @@
-// RUN: %clang_cc1 -std=c++17 -triple x86_64-linux-gnu 

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4861
+// We need correct types when the template-name is unresolved or when it
+// might be overloaded.
+if (!ResolvedTemplate)

And from the PR summary:

> namely mangling such template arguments as being cast to the parameter type 
> in the case where the template name is unresolved or overloaded

This phrasing worries me a little bit. Are you saying that you might mangle the 
name of `foo` in one way, when there's also a `foo` 
in scope, and in a different way, when there's not? That doesn't seem 
conforming. So I imagine it's more likely that I'm misunderstanding what you 
mean by "might be overloaded" / "is overloaded". Could you explain for my 
information, and perhaps also adjust the wording of these code comments to make 
the explanation less needed?

Specifically, I think it would be non-conforming if the TU

// https://godbolt.org/z/YjPqMd
template void foo();
char arr[6];
extern template void foo();  // #1
int main() { foo(); }

could not be linked against the TU

template int foo();
template void foo();  // is this "overloading"?
extern char arr[6];
template<> void foo() {}  // #2

because lines #1 and #2 disagreed about the way to mangle `foo`. (Again, 
I'm pretty sure you haven't made them disagree... but I remain unclear on 
what's meant by "overloading" in this PR, if it's //not// this.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added a subscriber: dexonsmith.
Herald added a project: clang.
rsmith requested review of this revision.

For the Itanium ABI, this implements the mangling rule suggested in
https://github.com/itanium-cxx-abi/cxx-abi/issues/47, namely mangling
such template arguments as being cast to the parameter type in the case
where the template name is unresolved or overloaded. This can cause a
mangling change for rare cases, where

- the template argument declaration is converted from its declared type to the 
type of the template parameter, and
- the template parameter either has a deduced type, is a parameter of a 
function tempate (or is somehow a resolved parameter of an unresolved template 
name).

However, such changes are necessary to avoid mangling collisions. The
ABI changes can be reversed with -fclang-abi-compat=11 or earlier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91488

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/clang-abi-compat.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -187,6 +187,11 @@
 int  = f(B<>());
 float  = f(B<>());
 
+void type_affects_identity(B<>) {}
+void type_affects_identity(B<(const int*)>) {}
+void type_affects_identity(B<(void*)>) {}
+void type_affects_identity(B<(const void*)>) {}
+
 // pointers to members
 template struct B {};
 template struct B {};
@@ -198,6 +203,12 @@
 char  = f(B<::p>());
 short  = f(B<::pp>());
 
+struct Y : X {};
+void type_affects_identity(B<::n>) {}
+void type_affects_identity(B<(int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+void type_affects_identity(B<(const int X::*)::n>) {}
+void type_affects_identity(B<(const int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+
 // A case where we need to do auto-deduction, and check whether the
 // resulting dependent types match during partial ordering. These
 // templates are not ordered due to the mismatching function parameter.
Index: clang/test/CodeGenCXX/mangle-template.cpp
===
--- clang/test/CodeGenCXX/mangle-template.cpp
+++ clang/test/CodeGenCXX/mangle-template.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++20 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s --check-prefixes=CHECK,CXX20
 // expected-no-diagnostics
 
 namespace test1 {
@@ -212,3 +213,39 @@
 template __make_integer_seq make<5>();
 // CHECK: define weak_odr {{.*}} @_ZN6test154makeILi5EEE18__make_integer_seqISt16integer_sequenceiXT_EEv(
 }
+
+#if __cplusplus >= 202002L
+namespace cxx20 {
+  template struct A {};
+  template struct B {};
+
+  int x;
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1xE(
+  void f(A<>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKiadL_ZNS_1xE(
+  void f(A<(const int*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPvadL_ZNS_1xE(
+  void f(A<(void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPvXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKvadL_ZNS_1xE(
+  void f(A<(const void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKvXadL_ZNS_1xE(
+  void f(B) {}
+
+  struct Q { int x; };
+
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1Q1xE(
+  void f(A<::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEiXadL_ZNS1_1xE
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvMNS_1QEKiadL_ZNS1_1xE(
+  void f(A<(const int Q::*)::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEKiXadL_ZNS1_1xE(
+  void f(B) {}
+}
+#endif
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -117,7 +117,7 @@
 template void f() {}
 
 // Union members.
-// CHECK: define weak_odr void @_Z1fIXL1vv(
+// CHECK: define weak_odr void @_Z1fIL1EEEvv(
 // FIXME: MSVC rejects this; check this is the mangling MSVC uses when they
 // start accepting.
 // MSABI: define {{.*}} @"??$f@$2TE@YAXXZ"
@@ -209,10