erik.pilkington created this revision.
erik.pilkington added a reviewer: rsmith.
Herald added a reviewer: javed.absar.
Herald added subscribers: dexonsmith, kristof.beyls.

The problem was with constructors that have parameters that refer to previous 
ones, such as `D` below:

  template <size_t> struct A {};
  template <class T> struct D {
    D(T t, A<sizeof(t)>);
  };
  int main() { D d(0, A<4>()); }

We were canonicalizing the type of A<sizeof(t)> to refer to the `t` in the 
constructor as opposed to the new one in the deduction guide. This is because 
ParmVarDecls are profiled based on their type and offset, so the two 
ParmVarDecls profiled to the same ID, despite them belonging to distinct 
functions. The fix for this here is to add a bit to ParmVarDecl, 
IsParamOfDeductionGuide, that tracks if this ParmVarDecl is "spliced" from 
another function. This makes references to the new ParmVarDecl not canonicalize 
to the previous ParmVarDecl.

This patch also fixes a closely related bug where newly created ParmVarDecls 
were not being added to the CurrentInstantiationScope, leading to an assert 
when we try to find it later.

rdar://41330135

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D49439

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp

Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===================================================================
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -348,6 +348,22 @@
   };
 }
 
+namespace rdar41330135 {
+template <int> struct A {};
+template <class T>
+struct S {
+  template <class U>
+  S(T a, U t, A<sizeof(t)>);
+};
+template <class T> struct D {
+  D(T t, A<sizeof(t)>);
+};
+int f() {
+  S s(0, 0, A<sizeof(int)>());
+  D d(0, A<sizeof(int)>());
+}
+}
+
 #else
 
 // expected-no-diagnostics
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1880,9 +1880,7 @@
                              MultiLevelTemplateArgumentList &Args) {
     TypeSourceInfo *OldDI = OldParam->getTypeSourceInfo();
     TypeSourceInfo *NewDI;
-    if (!Args.getNumLevels())
-      NewDI = OldDI;
-    else if (auto PackTL = OldDI->getTypeLoc().getAs<PackExpansionTypeLoc>()) {
+    if (auto PackTL = OldDI->getTypeLoc().getAs<PackExpansionTypeLoc>()) {
       // Expand out the one and only element in each inner pack.
       Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(SemaRef, 0);
       NewDI =
@@ -1912,9 +1910,7 @@
     // constructor.
     ExprResult NewDefArg;
     if (OldParam->hasDefaultArg()) {
-      NewDefArg = Args.getNumLevels()
-                      ? SemaRef.SubstExpr(OldParam->getDefaultArg(), Args)
-                      : OldParam->getDefaultArg();
+      NewDefArg = SemaRef.SubstExpr(OldParam->getDefaultArg(), Args);
       if (NewDefArg.isInvalid())
         return nullptr;
     }
@@ -1929,6 +1925,8 @@
                                                 NewDefArg.get());
     NewParam->setScopeInfo(OldParam->getFunctionScopeDepth(),
                            OldParam->getFunctionScopeIndex());
+    NewParam->setIsParamOfDeductionGuide();
+    SemaRef.CurrentInstantiationScope->InstantiatedLocal(OldParam, NewParam);
     return NewParam;
   }
 
Index: clang/lib/AST/StmtProfile.cpp
===================================================================
--- clang/lib/AST/StmtProfile.cpp
+++ clang/lib/AST/StmtProfile.cpp
@@ -110,6 +110,7 @@
           VisitType(Parm->getType());
           ID.AddInteger(Parm->getFunctionScopeDepth());
           ID.AddInteger(Parm->getFunctionScopeIndex());
+          ID.AddBoolean(Parm->isParamOfDeductionGuide());
           return;
         }
 
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -907,6 +907,10 @@
     /// declared.
     unsigned ScopeDepthOrObjCQuals : 7;
 
+    /// If this ParmVarDecl is a parameter of a deduction guide. This ensures
+    /// that we don't profile it the same as the parameter of the constructor.
+    unsigned IsParamOfDeductionGuide : 1;
+
     /// The number of parameters preceding this parameter in the
     /// function parameter scope in which it was declared.
     unsigned ParameterIndex : NumParameterIndexBits;
@@ -1544,6 +1548,7 @@
     assert(ParmVarDeclBits.DefaultArgKind == DAK_None);
     assert(ParmVarDeclBits.IsKNRPromoted == false);
     assert(ParmVarDeclBits.IsObjCMethodParam == false);
+    assert(ParmVarDeclBits.IsParamOfDeductionGuide == false);
     setDefaultArg(DefArg);
   }
 
@@ -1611,6 +1616,13 @@
     ParmVarDeclBits.IsKNRPromoted = promoted;
   }
 
+  void setIsParamOfDeductionGuide(bool Value = true) {
+    ParmVarDeclBits.IsParamOfDeductionGuide = Value;
+  }
+  bool isParamOfDeductionGuide() const {
+    return ParmVarDeclBits.IsParamOfDeductionGuide;
+  }
+
   Expr *getDefaultArg();
   const Expr *getDefaultArg() const {
     return const_cast<ParmVarDecl *>(this)->getDefaultArg();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D49439: [... Erik Pilkington via Phabricator via cfe-commits

Reply via email to