Re: [PATCH] D21030: [Sema] Fix rejects-valid where parameter pack was not expanded in type alias

2016-07-05 Thread Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL274566: [Sema] Fix a bug where pack expansion was not 
expanded in type alias (authored by epilk).

Changed prior to commit:
  http://reviews.llvm.org/D21030?vs=60908&id=62771#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D21030

Files:
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/p5.cpp

Index: cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -437,3 +437,35 @@
   template void g<>();
   template void g<1, 2, 3>();
 }
+
+template 
+int var_expr(Ts... ts);
+
+template 
+auto a_function(Ts... ts) -> decltype(var_expr(ts...));
+
+template 
+using partial = decltype(a_function);
+
+int use_partial() { partial n; }
+
+namespace PR26017 {
+template 
+struct Foo {};
+template 
+using FooAlias = Foo;
+
+template 
+using FooAliasAlias = FooAlias;
+
+template 
+void bar(const FooAlias &) {}
+
+int fn() {
+  FooAlias<> a;
+  bar(a);
+
+  FooAlias b;
+  bar(b);
+}
+}
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -3327,8 +3327,6 @@
 if (Out.isInvalid())
   return true;
 
-// FIXME: Can this happen? We should not try to expand the pack
-// in this case.
 if (Out.get()->containsUnexpandedParameterPack()) {
   Out = getDerived().RebuildPackExpansion(
   Out.get(), Expansion->getEllipsisLoc(), OrigNumExpansions);
@@ -4822,6 +4820,14 @@
   if (NewType.isNull())
 return true;
 
+  if (NewType->containsUnexpandedParameterPack()) {
+NewType =
+getSema().getASTContext().getPackExpansionType(NewType, None);
+
+if (NewType.isNull())
+  return true;
+  }
+
   if (ParamInfos)
 PInfos.set(OutParamTypes.size(), ParamInfos[i]);
   OutParamTypes.push_back(NewType);


Index: cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -437,3 +437,35 @@
   template void g<>();
   template void g<1, 2, 3>();
 }
+
+template 
+int var_expr(Ts... ts);
+
+template 
+auto a_function(Ts... ts) -> decltype(var_expr(ts...));
+
+template 
+using partial = decltype(a_function);
+
+int use_partial() { partial n; }
+
+namespace PR26017 {
+template 
+struct Foo {};
+template 
+using FooAlias = Foo;
+
+template 
+using FooAliasAlias = FooAlias;
+
+template 
+void bar(const FooAlias &) {}
+
+int fn() {
+  FooAlias<> a;
+  bar(a);
+
+  FooAlias b;
+  bar(b);
+}
+}
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -3327,8 +3327,6 @@
 if (Out.isInvalid())
   return true;
 
-// FIXME: Can this happen? We should not try to expand the pack
-// in this case.
 if (Out.get()->containsUnexpandedParameterPack()) {
   Out = getDerived().RebuildPackExpansion(
   Out.get(), Expansion->getEllipsisLoc(), OrigNumExpansions);
@@ -4822,6 +4820,14 @@
   if (NewType.isNull())
 return true;
 
+  if (NewType->containsUnexpandedParameterPack()) {
+NewType =
+getSema().getASTContext().getPackExpansionType(NewType, None);
+
+if (NewType.isNull())
+  return true;
+  }
+
   if (ParamInfos)
 PInfos.set(OutParamTypes.size(), ParamInfos[i]);
   OutParamTypes.push_back(NewType);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21030: [Sema] Fix rejects-valid where parameter pack was not expanded in type alias

2016-07-01 Thread Richard Smith via cfe-commits 
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM


http://reviews.llvm.org/D21030



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

Re: [PATCH] D21030: [Sema] Fix rejects-valid where parameter pack was not expanded in type alias

2016-06-27 Thread Erik Pilkington via cfe-commits 
erik.pilkington added a comment.

Ping!


http://reviews.llvm.org/D21030



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

Re: [PATCH] D21030: [Sema] Fix rejects-valid where parameter pack was not expanded in type alias

2016-06-15 Thread Erik Pilkington via cfe-commits 
erik.pilkington updated this revision to Diff 60908.
erik.pilkington added a comment.

This new patch replaces the call to `RebuildPackExpansion` with a direct call 
to `getPackExpansionType`. Also use `None` instead of `NumExpansions` as 
argument.

As far as the `assert(*NumExpansions == 1)`, I don't believe that is always the 
case. Consider:

  template struct Foo {};
  template using FooAlias = Foo;
  template using FooAliasAlias = FooAlias;

Here, the `Ts` in `FooAlias` are expanded into 2 `Us` in `FooAliasAlias`, 
meaning that there are 2 expansions that need to be separately rebuilt. I added 
this case to the test file.

Thanks for the review!


http://reviews.llvm.org/D21030

Files:
  lib/Sema/TreeTransform.h
  test/CXX/temp/temp.decls/temp.variadic/p5.cpp

Index: test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -437,3 +437,35 @@
   template void g<>();
   template void g<1, 2, 3>();
 }
+
+template 
+int var_expr(Ts... ts);
+
+template 
+auto a_function(Ts... ts) -> decltype(var_expr(ts...));
+
+template 
+using partial = decltype(a_function);
+
+int use_partial() { partial n; }
+
+namespace PR26017 {
+template 
+struct Foo {};
+template 
+using FooAlias = Foo;
+
+template 
+using FooAliasAlias = FooAlias;
+
+template 
+void bar(const FooAlias &) {}
+
+int fn() {
+  FooAlias<> a;
+  bar(a);
+
+  FooAlias b;
+  bar(b);
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3310,8 +3310,6 @@
 if (Out.isInvalid())
   return true;
 
-// FIXME: Can this happen? We should not try to expand the pack
-// in this case.
 if (Out.get()->containsUnexpandedParameterPack()) {
   Out = getDerived().RebuildPackExpansion(
   Out.get(), Expansion->getEllipsisLoc(), OrigNumExpansions);
@@ -4780,6 +4778,14 @@
   if (NewType.isNull())
 return true;
 
+  if (NewType->containsUnexpandedParameterPack()) {
+NewType =
+getSema().getASTContext().getPackExpansionType(NewType, None);
+
+if (NewType.isNull())
+  return true;
+  }
+
   if (ParamInfos)
 PInfos.set(OutParamTypes.size(), ParamInfos[i]);
   OutParamTypes.push_back(NewType);


Index: test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -437,3 +437,35 @@
   template void g<>();
   template void g<1, 2, 3>();
 }
+
+template 
+int var_expr(Ts... ts);
+
+template 
+auto a_function(Ts... ts) -> decltype(var_expr(ts...));
+
+template 
+using partial = decltype(a_function);
+
+int use_partial() { partial n; }
+
+namespace PR26017 {
+template 
+struct Foo {};
+template 
+using FooAlias = Foo;
+
+template 
+using FooAliasAlias = FooAlias;
+
+template 
+void bar(const FooAlias &) {}
+
+int fn() {
+  FooAlias<> a;
+  bar(a);
+
+  FooAlias b;
+  bar(b);
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3310,8 +3310,6 @@
 if (Out.isInvalid())
   return true;
 
-// FIXME: Can this happen? We should not try to expand the pack
-// in this case.
 if (Out.get()->containsUnexpandedParameterPack()) {
   Out = getDerived().RebuildPackExpansion(
   Out.get(), Expansion->getEllipsisLoc(), OrigNumExpansions);
@@ -4780,6 +4778,14 @@
   if (NewType.isNull())
 return true;
 
+  if (NewType->containsUnexpandedParameterPack()) {
+NewType =
+getSema().getASTContext().getPackExpansionType(NewType, None);
+
+if (NewType.isNull())
+  return true;
+  }
+
   if (ParamInfos)
 PInfos.set(OutParamTypes.size(), ParamInfos[i]);
   OutParamTypes.push_back(NewType);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21030: [Sema] Fix rejects-valid where parameter pack was not expanded in type alias

2016-06-14 Thread Faisal Vali via cfe-commits 
faisalv added a comment.

Thanks for working on this bug too!



Comment at: lib/Sema/TreeTransform.h:4784
@@ +4783,3 @@
+NewType = getDerived().RebuildPackExpansionType(
+NewType, SourceRange(), Loc, NumExpansions);
+if (NewType.isNull())

Any thoughts on the value of threading EllipsisLoc and PatternRange into this 
function (they are used primarily for a diagnostic that is triggered in Sema if 
NewType does not contain an unexpandedparameter pack - which would never get 
triggered here, so it would be ok to pass in invalid sourelocations and 
sourceranges with an appropriate comment here) - but the fact that 
RebuildPackExpandionType is a customization point, it might not be a bad idea 
to preserve them)?
Alternatively, we could forego calling RebuildPackExpansionType - since OldType 
is decomposed directly into a PackExpansionType -- NewType could be recomposed 
directly via Context.getPackExpansionType(NewType, NumExpansions=0)?

Which brings me to - when we create the new pack expansion type, shouldn't the 
NumExpansions be reset to 0?  The fact that we will have a 
SubstTemplateParameterType replacing the TemplateParameterType within the 
pattern might influence the answer to that...

Also - we should be able to assert here that *NumExpansions should be one?  Can 
you think of a case that it wouldn't be?



http://reviews.llvm.org/D21030



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

Re: [PATCH] D21030: [Sema] Fix rejects-valid where parameter pack was not expanded in type alias

2016-06-13 Thread Erik Pilkington via cfe-commits 
erik.pilkington added a comment.

Ping!


http://reviews.llvm.org/D21030



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

[PATCH] D21030: [Sema] Fix rejects-valid where parameter pack was not expanded in type alias

2016-06-06 Thread Erik Pilkington via cfe-commits 
erik.pilkington created this revision.
erik.pilkington added a reviewer: rsmith.
erik.pilkington added a subscriber: cfe-commits.

Clang rejects the following valid code:

```
template  struct Foo {};
template  using FooAlias = Foo;
template  int bar(FooAlias);
int main() { bar(FooAlias<>()); }
```

The problem is that in the substitution of `FooAlias` during the parsing 
of bar, the pack expansion in `Foo` is transformed into a 
parameter with the unexpanded flag set, as opposed to another pack expansion. 
This leads to deduction failing in `main`, which is incorrect. The fix is to 
expand the parameter pack when needed.

Fixes PR25250 & PR26017.

Incidentally, there was an incorrect comment in a very similar function that 
said that a path wasn't reachable. This isn't the case, although no existing 
test case exercised it, so I added one that did. Please let me know if it would 
be cleaner to commit that separately!

Thanks!

http://reviews.llvm.org/D21030

Files:
  lib/Sema/TreeTransform.h
  test/CXX/temp/temp.decls/temp.variadic/p5.cpp

Index: test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -437,3 +437,32 @@
   template void g<>();
   template void g<1, 2, 3>();
 }
+
+template 
+int var_expr(Ts... ts);
+
+template 
+auto a_function(Ts... ts) -> decltype(var_expr(ts...));
+
+template 
+using partial = decltype(a_function);
+
+int use_partial() { partial n; }
+
+namespace PR26017 {
+template 
+struct Foo {};
+template 
+using FooAlias = Foo;
+
+template 
+void bar(const FooAlias &) {}
+
+int fn() {
+  FooAlias<> a;
+  bar(a);
+
+  FooAlias b;
+  bar(b);
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3311,8 +3311,6 @@
 if (Out.isInvalid())
   return true;
 
-// FIXME: Can this happen? We should not try to expand the pack
-// in this case.
 if (Out.get()->containsUnexpandedParameterPack()) {
   Out = getDerived().RebuildPackExpansion(
   Out.get(), Expansion->getEllipsisLoc(), OrigNumExpansions);
@@ -4781,6 +4779,13 @@
   if (NewType.isNull())
 return true;
 
+  if (NewType->containsUnexpandedParameterPack()) {
+NewType = getDerived().RebuildPackExpansionType(
+NewType, SourceRange(), Loc, NumExpansions);
+if (NewType.isNull())
+  return true;
+  }
+
   if (ParamInfos)
 PInfos.set(OutParamTypes.size(), ParamInfos[i]);
   OutParamTypes.push_back(NewType);


Index: test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -437,3 +437,32 @@
   template void g<>();
   template void g<1, 2, 3>();
 }
+
+template 
+int var_expr(Ts... ts);
+
+template 
+auto a_function(Ts... ts) -> decltype(var_expr(ts...));
+
+template 
+using partial = decltype(a_function);
+
+int use_partial() { partial n; }
+
+namespace PR26017 {
+template 
+struct Foo {};
+template 
+using FooAlias = Foo;
+
+template 
+void bar(const FooAlias &) {}
+
+int fn() {
+  FooAlias<> a;
+  bar(a);
+
+  FooAlias b;
+  bar(b);
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3311,8 +3311,6 @@
 if (Out.isInvalid())
   return true;
 
-// FIXME: Can this happen? We should not try to expand the pack
-// in this case.
 if (Out.get()->containsUnexpandedParameterPack()) {
   Out = getDerived().RebuildPackExpansion(
   Out.get(), Expansion->getEllipsisLoc(), OrigNumExpansions);
@@ -4781,6 +4779,13 @@
   if (NewType.isNull())
 return true;
 
+  if (NewType->containsUnexpandedParameterPack()) {
+NewType = getDerived().RebuildPackExpansionType(
+NewType, SourceRange(), Loc, NumExpansions);
+if (NewType.isNull())
+  return true;
+  }
+
   if (ParamInfos)
 PInfos.set(OutParamTypes.size(), ParamInfos[i]);
   OutParamTypes.push_back(NewType);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits