[PATCH] D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316778: [Sema] Fix an assert-on-invalid by avoiding function 
template specialisation (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D37341?vs=119967=120658#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37341

Files:
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/test/SemaTemplate/deduction-crash.cpp
  cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp


Index: cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp
===
--- cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp
+++ cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp
@@ -38,24 +38,20 @@
 
   template
   template
-  void Baz::bar() { // expected-note {{couldn't infer template argument 
'N'}}
+  void Baz::bar() {
   }
 
-  // FIXME: We shouldn't try to match this against a prior declaration if
-  // template parameter matching failed.
   template
-  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an 
unspecialized template}} \
-  // expected-error {{no function template matches}}
+  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an 
unspecialized template}}
   }
 }
 
 namespace PR19340 {
 template struct Helper {
-  template static void func(const T *m) {} // expected-note {{failed 
template argument deduction}}
+  template static void func(const T *m) {}
 };
 
-template void Helper::func<2>() {} // expected-error {{cannot 
specialize a member}} \
-  // expected-error {{no 
function template matches}}
+template void Helper::func<2>() {} // expected-error {{cannot 
specialize a member}}
 }
 
 namespace SpecLoc {
Index: cfe/trunk/test/SemaTemplate/deduction-crash.cpp
===
--- cfe/trunk/test/SemaTemplate/deduction-crash.cpp
+++ cfe/trunk/test/SemaTemplate/deduction-crash.cpp
@@ -144,3 +144,20 @@
   template int n; // expected-error +{{}} 
expected-note {{}}
   int k = n;
 }
+
+namespace deduceFunctionSpecializationForInvalidOutOfLineFunction {
+
+template 
+struct SourceSelectionRequirement {
+  template
+  OutputT evaluateSelectionRequirement(InputT &) {
+  }
+};
+
+template 
+OutputT SourceSelectionRequirement::
+evaluateSelectionRequirement(InputT &) { // expected-error 
{{cannot specialize a member of an unspecialized template}}
+  return Value;
+}
+
+}
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -8962,10 +8962,10 @@
   diag::ext_function_specialization_in_class :
   diag::err_function_specialization_in_class)
   << NewFD->getDeclName();
-  } else if (CheckFunctionTemplateSpecialization(NewFD,
-  (HasExplicitTemplateArgs ? 
-   : nullptr),
- Previous))
+  } else if (!NewFD->isInvalidDecl() &&
+ CheckFunctionTemplateSpecialization(
+ NewFD, (HasExplicitTemplateArgs ?  : 
nullptr),
+ Previous))
 NewFD->setInvalidDecl();
 
   // C++ [dcl.stc]p1:


Index: cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp
===
--- cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp
+++ cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp
@@ -38,24 +38,20 @@
 
   template
   template
-  void Baz::bar() { // expected-note {{couldn't infer template argument 'N'}}
+  void Baz::bar() {
   }
 
-  // FIXME: We shouldn't try to match this against a prior declaration if
-  // template parameter matching failed.
   template
-  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an unspecialized template}} \
-  // expected-error {{no function template matches}}
+  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an unspecialized template}}
   }
 }
 
 namespace PR19340 {
 template struct Helper {
-  template static void func(const T *m) {} // expected-note {{failed template argument deduction}}
+  template static void func(const T *m) {}
 };
 
-template void Helper::func<2>() {} // expected-error {{cannot specialize a member}} \
-  // expected-error {{no function template matches}}
+template void Helper::func<2>() {} // expected-error {{cannot specialize a member}}
 }
 
 namespace SpecLoc {
Index: cfe/trunk/test/SemaTemplate/deduction-crash.cpp
===
--- cfe/trunk/test/SemaTemplate/deduction-crash.cpp
+++ 

[PATCH] D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 119967.
arphaman added a comment.

Use just the `isInvalidDecl` check.


Repository:
  rL LLVM

https://reviews.llvm.org/D37341

Files:
  lib/Sema/SemaDecl.cpp
  test/SemaTemplate/deduction-crash.cpp
  test/SemaTemplate/explicit-specialization-member.cpp


Index: test/SemaTemplate/explicit-specialization-member.cpp
===
--- test/SemaTemplate/explicit-specialization-member.cpp
+++ test/SemaTemplate/explicit-specialization-member.cpp
@@ -38,24 +38,20 @@
 
   template
   template
-  void Baz::bar() { // expected-note {{couldn't infer template argument 
'N'}}
+  void Baz::bar() {
   }
 
-  // FIXME: We shouldn't try to match this against a prior declaration if
-  // template parameter matching failed.
   template
-  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an 
unspecialized template}} \
-  // expected-error {{no function template matches}}
+  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an 
unspecialized template}}
   }
 }
 
 namespace PR19340 {
 template struct Helper {
-  template static void func(const T *m) {} // expected-note {{failed 
template argument deduction}}
+  template static void func(const T *m) {}
 };
 
-template void Helper::func<2>() {} // expected-error {{cannot 
specialize a member}} \
-  // expected-error {{no 
function template matches}}
+template void Helper::func<2>() {} // expected-error {{cannot 
specialize a member}}
 }
 
 namespace SpecLoc {
Index: test/SemaTemplate/deduction-crash.cpp
===
--- test/SemaTemplate/deduction-crash.cpp
+++ test/SemaTemplate/deduction-crash.cpp
@@ -144,3 +144,20 @@
   template int n; // expected-error +{{}} 
expected-note {{}}
   int k = n;
 }
+
+namespace deduceFunctionSpecializationForInvalidOutOfLineFunction {
+
+template 
+struct SourceSelectionRequirement {
+  template
+  OutputT evaluateSelectionRequirement(InputT &) {
+  }
+};
+
+template 
+OutputT SourceSelectionRequirement::
+evaluateSelectionRequirement(InputT &) { // expected-error 
{{cannot specialize a member of an unspecialized template}}
+  return Value;
+}
+
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8962,10 +8962,10 @@
   diag::ext_function_specialization_in_class :
   diag::err_function_specialization_in_class)
   << NewFD->getDeclName();
-  } else if (CheckFunctionTemplateSpecialization(NewFD,
-  (HasExplicitTemplateArgs ? 
-   : nullptr),
- Previous))
+  } else if (!NewFD->isInvalidDecl() &&
+ CheckFunctionTemplateSpecialization(
+ NewFD, (HasExplicitTemplateArgs ?  : 
nullptr),
+ Previous))
 NewFD->setInvalidDecl();
 
   // C++ [dcl.stc]p1:


Index: test/SemaTemplate/explicit-specialization-member.cpp
===
--- test/SemaTemplate/explicit-specialization-member.cpp
+++ test/SemaTemplate/explicit-specialization-member.cpp
@@ -38,24 +38,20 @@
 
   template
   template
-  void Baz::bar() { // expected-note {{couldn't infer template argument 'N'}}
+  void Baz::bar() {
   }
 
-  // FIXME: We shouldn't try to match this against a prior declaration if
-  // template parameter matching failed.
   template
-  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an unspecialized template}} \
-  // expected-error {{no function template matches}}
+  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an unspecialized template}}
   }
 }
 
 namespace PR19340 {
 template struct Helper {
-  template static void func(const T *m) {} // expected-note {{failed template argument deduction}}
+  template static void func(const T *m) {}
 };
 
-template void Helper::func<2>() {} // expected-error {{cannot specialize a member}} \
-  // expected-error {{no function template matches}}
+template void Helper::func<2>() {} // expected-error {{cannot specialize a member}}
 }
 
 namespace SpecLoc {
Index: test/SemaTemplate/deduction-crash.cpp
===
--- test/SemaTemplate/deduction-crash.cpp
+++ test/SemaTemplate/deduction-crash.cpp
@@ -144,3 +144,20 @@
   template int n; // expected-error +{{}} expected-note {{}}
   int k = n;
 }
+
+namespace deduceFunctionSpecializationForInvalidOutOfLineFunction {
+
+template 
+struct SourceSelectionRequirement {
+  template
+  OutputT evaluateSelectionRequirement(InputT &) {
+  }
+};
+
+template 

[PATCH] D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments

[Alex Lorenz via Phabricator via cfe-commits]

arphaman marked an inline comment as done.
arphaman added a comment.

In https://reviews.llvm.org/D37341#869042, @vsapsai wrote:

> Does your fix work for deeper nesting too (e.g. template in template in 
> template)? Looks like it should, just want to confirm.


Yes.

> Are there other places where you need to avoid calling 
> `DeduceTemplateArguments` due to templates depth mismatch? 
> `CheckDependentFunctionTemplateSpecialization` should be OK as it's not 
> deducing template arguments. But I'm not sure about 
> `CheckMemberSpecialization`. It doesn't call `DeduceTemplateArguments` 
> directly but I haven't dug deep enough to confirm it's not performing 
> deduction indirectly.

The problem only seems to happen when the TPL is fabricated. I didn't see other 
potential places where such an issue might happen.




Comment at: lib/Sema/SemaDecl.cpp:8880-8881
   << NewFD->getDeclName();
-  } else if (CheckFunctionTemplateSpecialization(NewFD,
+  } else if (!HasFabricatedTemplateSpecializationTemplateParams &&
+ CheckFunctionTemplateSpecialization(NewFD,
   (HasExplicitTemplateArgs ? 

vsapsai wrote:
> Will something more general like
> 
> !NewFD->isInvalidDecl() && CheckFunctionTemplateSpecialization(...)
> 
> work here? Because if matching template parameters to scope specifier fails, 
> `NewFD` is marked as invalid decl and seems like we can use that.
Yeah, good idea.


Repository:
  rL LLVM

https://reviews.llvm.org/D37341



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

[PATCH] D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Does your fix work for deeper nesting too (e.g. template in template in 
template)? Looks like it should, just want to confirm.

Are there other places where you need to avoid calling 
`DeduceTemplateArguments` due to templates depth mismatch? 
`CheckDependentFunctionTemplateSpecialization` should be OK as it's not 
deducing template arguments. But I'm not sure about 
`CheckMemberSpecialization`. It doesn't call `DeduceTemplateArguments` directly 
but I haven't dug deep enough to confirm it's not performing deduction 
indirectly.




Comment at: lib/Sema/SemaDecl.cpp:8307-8308
 isFunctionTemplateSpecialization = true;
+if (Invalid && TemplateParams->getLAngleLoc().isInvalid())
+  HasFabricatedTemplateSpecializationTemplateParams = true;
 // For source fidelity, store all the template param lists.

Checking if angle location is invalid looks suspicious. It can be my lack of 
knowledge but I expect various locations not to convey sema information.



Comment at: lib/Sema/SemaDecl.cpp:8880-8881
   << NewFD->getDeclName();
-  } else if (CheckFunctionTemplateSpecialization(NewFD,
+  } else if (!HasFabricatedTemplateSpecializationTemplateParams &&
+ CheckFunctionTemplateSpecialization(NewFD,
   (HasExplicitTemplateArgs ? 

Will something more general like

!NewFD->isInvalidDecl() && CheckFunctionTemplateSpecialization(...)

work here? Because if matching template parameters to scope specifier fails, 
`NewFD` is marked as invalid decl and seems like we can use that.


Repository:
  rL LLVM

https://reviews.llvm.org/D37341



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

[PATCH] D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.

This patch fixes an an assertion that previously occurred for the following 
sample:

  template 
  struct SourceSelectionRequirement {
  
template
OutputT evaluateSelectionRequirement(InputT &) {
}
  };
  
  template 
  OutputT SourceSelectionRequirement::evaluateSelectionRequirement(InputT &) {
  return Value;
  }

Clang asserted when it was trying to deduce the function template 
specialisation for `evaluateSelectionRequirement`, because it was trying to do 
deduction with the template specialisation `` and `template ` which have different template depth.

I initially fixed the issue by setting the right depth, during the deduction, 
but wasn't happy with the results (we produced two errors, the second one 
complained about failed deduction). Thus I changed my approach to the one 
that's presented in this patch - we can detect if the template parameters are 
fabricated and then avoid the deduction. This approach avoids the second error 
while fixing the assertion. It also fixed a redundant error `FIXME` in 
`SemaTemplate/explicit-specialization-member.cpp` (Basically removed a 
redundant deduction error).

Thanks for taking a look


Repository:
  rL LLVM

https://reviews.llvm.org/D37341

Files:
  lib/Sema/SemaDecl.cpp
  test/SemaTemplate/deduction-crash.cpp
  test/SemaTemplate/explicit-specialization-member.cpp


Index: test/SemaTemplate/explicit-specialization-member.cpp
===
--- test/SemaTemplate/explicit-specialization-member.cpp
+++ test/SemaTemplate/explicit-specialization-member.cpp
@@ -38,24 +38,20 @@
 
   template
   template
-  void Baz::bar() { // expected-note {{couldn't infer template argument 
'N'}}
+  void Baz::bar() {
   }
 
-  // FIXME: We shouldn't try to match this against a prior declaration if
-  // template parameter matching failed.
   template
-  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an 
unspecialized template}} \
-  // expected-error {{no function template matches}}
+  void Baz::bar<0>() { // expected-error {{cannot specialize a member of an 
unspecialized template}}
   }
 }
 
 namespace PR19340 {
 template struct Helper {
-  template static void func(const T *m) {} // expected-note {{failed 
template argument deduction}}
+  template static void func(const T *m) {}
 };
 
-template void Helper::func<2>() {} // expected-error {{cannot 
specialize a member}} \
-  // expected-error {{no 
function template matches}}
+template void Helper::func<2>() {} // expected-error {{cannot 
specialize a member}}
 }
 
 namespace SpecLoc {
Index: test/SemaTemplate/deduction-crash.cpp
===
--- test/SemaTemplate/deduction-crash.cpp
+++ test/SemaTemplate/deduction-crash.cpp
@@ -144,3 +144,20 @@
   template int n; // expected-error +{{}} 
expected-note {{}}
   int k = n;
 }
+
+namespace deduceFunctionSpecializationForInvalidOutOfLineFunction {
+
+template 
+struct SourceSelectionRequirement {
+  template
+  OutputT evaluateSelectionRequirement(InputT &) {
+  }
+};
+
+template 
+OutputT SourceSelectionRequirement::
+evaluateSelectionRequirement(InputT &) { // expected-error 
{{cannot specialize a member of an unspecialized template}}
+  return Value;
+}
+
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8195,6 +8195,7 @@
   FunctionTemplateDecl *FunctionTemplate = nullptr;
   bool isMemberSpecialization = false;
   bool isFunctionTemplateSpecialization = false;
+  bool HasFabricatedTemplateSpecializationTemplateParams = false;
 
   bool isDependentClassScopeExplicitSpecialization = false;
   bool HasExplicitTemplateArgs = false;
@@ -8303,6 +8304,8 @@
   } else {
 // This is a function template specialization.
 isFunctionTemplateSpecialization = true;
+if (Invalid && TemplateParams->getLAngleLoc().isInvalid())
+  HasFabricatedTemplateSpecializationTemplateParams = true;
 // For source fidelity, store all the template param lists.
 if (TemplateParamLists.size() > 0)
   NewFD->setTemplateParameterListsInfo(Context, TemplateParamLists);
@@ -8874,7 +8877,8 @@
   diag::ext_function_specialization_in_class :
   diag::err_function_specialization_in_class)
   << NewFD->getDeclName();
-  } else if (CheckFunctionTemplateSpecialization(NewFD,
+  } else if (!HasFabricatedTemplateSpecializationTemplateParams &&
+ CheckFunctionTemplateSpecialization(NewFD,
   (HasExplicitTemplateArgs ? 
: nullptr),
  Previous))


Index: