Author: Saar Raz
Date: 2020-01-30T20:45:44+02:00
New Revision: c83d9bedc0cc430dc620e7a807daeb985d390325

URL: 
https://github.com/llvm/llvm-project/commit/c83d9bedc0cc430dc620e7a807daeb985d390325
DIFF: 
https://github.com/llvm/llvm-project/commit/c83d9bedc0cc430dc620e7a807daeb985d390325.diff

LOG: [Concept] Fix incorrect check for containsUnexpandedParameterPack in CSE

We previously checked for containsUnexpandedParameterPack in CSEs by observing 
the property
in the converted arguments of the CSE. This may not work if the argument is an 
expanded
type-alias that contains a pack-expansion (see added test).

Check the as-written arguments when determining containsUnexpandedParameterPack 
and isInstantiationDependent.

Added: 
    

Modified: 
    clang/include/clang/AST/ExprConcepts.h
    clang/lib/AST/ASTContext.cpp
    clang/lib/AST/ExprConcepts.cpp
    clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ExprConcepts.h 
b/clang/include/clang/AST/ExprConcepts.h
index 2a64326e8604..271d487e2fc9 100644
--- a/clang/include/clang/AST/ExprConcepts.h
+++ b/clang/include/clang/AST/ExprConcepts.h
@@ -63,6 +63,12 @@ class ConceptSpecializationExpr final : public Expr, public 
ConceptReference,
                             ArrayRef<TemplateArgument> ConvertedArgs,
                             const ConstraintSatisfaction *Satisfaction);
 
+  ConceptSpecializationExpr(const ASTContext &C, ConceptDecl *NamedConcept,
+                            ArrayRef<TemplateArgument> ConvertedArgs,
+                            const ConstraintSatisfaction *Satisfaction,
+                            bool Dependent,
+                            bool ContainsUnexpandedParameterPack);
+
   ConceptSpecializationExpr(EmptyShell Empty, unsigned NumTemplateArgs);
 
 public:
@@ -75,6 +81,13 @@ class ConceptSpecializationExpr final : public Expr, public 
ConceptReference,
          ArrayRef<TemplateArgument> ConvertedArgs,
          const ConstraintSatisfaction *Satisfaction);
 
+  static ConceptSpecializationExpr *
+  Create(const ASTContext &C, ConceptDecl *NamedConcept,
+         ArrayRef<TemplateArgument> ConvertedArgs,
+         const ConstraintSatisfaction *Satisfaction,
+         bool Dependent,
+         bool ContainsUnexpandedParameterPack);
+
   static ConceptSpecializationExpr *
   Create(ASTContext &C, EmptyShell Empty, unsigned NumTemplateArgs);
 

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 72969490e32d..c80d3948d003 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -731,12 +731,8 @@ canonicalizeImmediatelyDeclaredConstraint(const ASTContext 
&C, Expr *IDC,
       NewConverted.push_back(Arg);
   }
   Expr *NewIDC = ConceptSpecializationExpr::Create(
-      C, NestedNameSpecifierLoc(), /*TemplateKWLoc=*/SourceLocation(),
-      CSE->getConceptNameInfo(), /*FoundDecl=*/CSE->getNamedConcept(),
-      CSE->getNamedConcept(),
-      // Actually canonicalizing a TemplateArgumentLoc is 
diff icult so we
-      // simply omit the ArgsAsWritten
-      /*ArgsAsWritten=*/nullptr, NewConverted, nullptr);
+      C, CSE->getNamedConcept(), NewConverted, nullptr,
+      CSE->isInstantiationDependent(), CSE->containsUnexpandedParameterPack());
 
   if (auto *OrigFold = dyn_cast<CXXFoldExpr>(IDC))
     NewIDC = new (C) CXXFoldExpr(OrigFold->getType(), SourceLocation(), NewIDC,

diff  --git a/clang/lib/AST/ExprConcepts.cpp b/clang/lib/AST/ExprConcepts.cpp
index 76d57ed5d5b1..b5a3686dc99a 100644
--- a/clang/lib/AST/ExprConcepts.cpp
+++ b/clang/lib/AST/ExprConcepts.cpp
@@ -46,24 +46,12 @@ ConceptSpecializationExpr::ConceptSpecializationExpr(const 
ASTContext &C,
                    ASTConstraintSatisfaction::Create(C, *Satisfaction) :
                    nullptr) {
   setTemplateArguments(ConvertedArgs);
-}
-
-ConceptSpecializationExpr::ConceptSpecializationExpr(EmptyShell Empty,
-    unsigned NumTemplateArgs)
-    : Expr(ConceptSpecializationExprClass, Empty), ConceptReference(),
-      NumTemplateArgs(NumTemplateArgs) { }
-
-void ConceptSpecializationExpr::setTemplateArguments(
-    ArrayRef<TemplateArgument> Converted) {
-  assert(Converted.size() == NumTemplateArgs);
-  std::uninitialized_copy(Converted.begin(), Converted.end(),
-                          getTrailingObjects<TemplateArgument>());
   bool IsInstantiationDependent = false;
   bool ContainsUnexpandedParameterPack = false;
-  for (const TemplateArgument& Arg : Converted) {
-    if (Arg.isInstantiationDependent())
+  for (const TemplateArgumentLoc& ArgLoc : ArgsAsWritten->arguments()) {
+    if (ArgLoc.getArgument().isInstantiationDependent())
       IsInstantiationDependent = true;
-    if (Arg.containsUnexpandedParameterPack())
+    if (ArgLoc.getArgument().containsUnexpandedParameterPack())
       ContainsUnexpandedParameterPack = true;
     if (ContainsUnexpandedParameterPack && IsInstantiationDependent)
       break;
@@ -80,6 +68,18 @@ void ConceptSpecializationExpr::setTemplateArguments(
          "should not be value-dependent");
 }
 
+ConceptSpecializationExpr::ConceptSpecializationExpr(EmptyShell Empty,
+    unsigned NumTemplateArgs)
+    : Expr(ConceptSpecializationExprClass, Empty), ConceptReference(),
+      NumTemplateArgs(NumTemplateArgs) { }
+
+void ConceptSpecializationExpr::setTemplateArguments(
+    ArrayRef<TemplateArgument> Converted) {
+  assert(Converted.size() == NumTemplateArgs);
+  std::uninitialized_copy(Converted.begin(), Converted.end(),
+                          getTrailingObjects<TemplateArgument>());
+}
+
 ConceptSpecializationExpr *
 ConceptSpecializationExpr::Create(const ASTContext &C,
                                   NestedNameSpecifierLoc NNS,
@@ -98,6 +98,39 @@ ConceptSpecializationExpr::Create(const ASTContext &C,
                                                 ConvertedArgs, Satisfaction);
 }
 
+ConceptSpecializationExpr::ConceptSpecializationExpr(
+    const ASTContext &C, ConceptDecl *NamedConcept,
+    ArrayRef<TemplateArgument> ConvertedArgs,
+    const ConstraintSatisfaction *Satisfaction, bool Dependent,
+    bool ContainsUnexpandedParameterPack)
+    : Expr(ConceptSpecializationExprClass, C.BoolTy, VK_RValue, OK_Ordinary,
+           /*TypeDependent=*/false,
+           /*ValueDependent=*/!Satisfaction, Dependent,
+           ContainsUnexpandedParameterPack),
+      ConceptReference(NestedNameSpecifierLoc(), SourceLocation(),
+                       DeclarationNameInfo(), NamedConcept,
+                       NamedConcept, nullptr),
+      NumTemplateArgs(ConvertedArgs.size()),
+      Satisfaction(Satisfaction ?
+                   ASTConstraintSatisfaction::Create(C, *Satisfaction) :
+                   nullptr) {
+  setTemplateArguments(ConvertedArgs);
+}
+
+ConceptSpecializationExpr *
+ConceptSpecializationExpr::Create(const ASTContext &C,
+                                  ConceptDecl *NamedConcept,
+                                  ArrayRef<TemplateArgument> ConvertedArgs,
+                                  const ConstraintSatisfaction *Satisfaction,
+                                  bool Dependent,
+                                  bool ContainsUnexpandedParameterPack) {
+  void *Buffer = C.Allocate(totalSizeToAlloc<TemplateArgument>(
+                                ConvertedArgs.size()));
+  return new (Buffer) ConceptSpecializationExpr(
+      C, NamedConcept, ConvertedArgs, Satisfaction, Dependent,
+      ContainsUnexpandedParameterPack);
+}
+
 ConceptSpecializationExpr *
 ConceptSpecializationExpr::Create(ASTContext &C, EmptyShell Empty,
                                   unsigned NumTemplateArgs) {

diff  --git a/clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp 
b/clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp
index fb3978240af2..30ec2a3ab70f 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp
@@ -183,3 +183,18 @@ concept C8 = sizeof(T) > sizeof(U);
 template<typename... T>
 constexpr bool B8 = C8<T...>;
 // expected-error@-1{{pack expansion used as argument for non-pack parameter 
of concept}}
+
+
+// Make sure we correctly check for containsUnexpandedParameterPack
+
+template<typename T>
+concept C9 = true;
+
+template <typename Fn, typename... Args>
+using invoke = typename Fn::template invoke<Args...>;
+
+template <typename C, typename... L>
+// The converted argument here will not containsUnexpandedParameterPack, but 
the
+// as-written one will.
+requires (C9<invoke<C, L>> &&...)
+struct S { };


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

Reply via email to