Re: r257831 - Refactor template type diffing

[Richard Smith via cfe-commits]

On Thu, Jan 14, 2016 at 2:57 PM, Richard Trieu via cfe-commits
 wrote:
> Author: rtrieu
> Date: Thu Jan 14 16:56:39 2016
> New Revision: 257831
>
> URL: http://llvm.org/viewvc/llvm-project?rev=257831=rev
> Log:
> Refactor template type diffing
>
> 1) Instead of using pairs of From/To* fields, combine fields into a struct
> TemplateArgInfo and have two in each DiffNode.
> 2) Use default initialization in DiffNode so that the constructor shows the
> only field that is initialized differently on construction.
> 3) Use Set and Get functions per each DiffKind to make sure all fields for the
> diff is set.  In one case, the Expr fields were not set.
> 4) Don't print boolean literals for boolean template arguments.  This prevents
> printing 'false aka 0'
>
> Only #3 has a functional change, which is reflected in the test change.
>
> Modified:
> cfe/trunk/lib/AST/ASTDiagnostic.cpp
> cfe/trunk/test/Misc/diag-template-diffing.cpp
>
> Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDiagnostic.cpp?rev=257831=257830=257831=diff
> ==
> --- cfe/trunk/lib/AST/ASTDiagnostic.cpp (original)
> +++ cfe/trunk/lib/AST/ASTDiagnostic.cpp Thu Jan 14 16:56:39 2016
> @@ -491,82 +491,67 @@ class TemplateDiff {
>/// DiffTree - A tree representation the differences between two types.
>class DiffTree {
>public:
> -/// DiffKind - The difference in a DiffNode and which fields are used.
> +/// DiffKind - The difference in a DiffNode.  Fields of
> +/// TemplateArgumentInfo needed by each difference can be found in the
> +/// Set* and Get* functions.
>  enum DiffKind {
>/// Incomplete or invalid node.
>Invalid,
> -  /// Another level of templates, uses TemplateDecl and Qualifiers
> +  /// Another level of templates, requires that

... requires that what?

>Template,
> -  /// Type difference, uses QualType
> +  /// Type difference, all type differences except those falling under
> +  /// the Template difference.
>Type,
> -  /// Expression difference, uses Expr
> +  /// Expression difference, this is only when both arguments are
> +  /// expressions.  If one argument is an expression and the other is
> +  /// Integer or Declaration, then use that diff type instead.
>Expression,
> -  /// Template argument difference, uses TemplateDecl
> +  /// Template argument difference
>TemplateTemplate,
> -  /// Integer difference, uses APSInt and Expr
> +  /// Integer difference
>Integer,
> -  /// Declaration difference, uses ValueDecl
> +  /// Declaration difference, nullptr arguments are included here
>Declaration
>  };
> +
>private:
> +/// TemplateArgumentInfo - All the information needed to pretty print
> +/// a template argument.  See the Set* and Get* functions to see which
> +/// fields are used for each DiffKind.
> +struct TemplateArgumentInfo {
> +  QualType ArgType;
> +  Qualifiers Qual;
> +  llvm::APSInt Val;
> +  bool IsValidInt = false;
> +  Expr *ArgExpr = nullptr;
> +  TemplateDecl *TD = nullptr;
> +  ValueDecl *VD = nullptr;
> +  bool NeedAddressOf = false;
> +  bool IsNullPtr = false;
> +  bool IsDefault = false;
> +};
> +
>  /// DiffNode - The root node stores the original type.  Each child node
>  /// stores template arguments of their parents.  For templated types, the
>  /// template decl is also stored.
>  struct DiffNode {
> -  DiffKind Kind;
> +  DiffKind Kind = Invalid;
>
>/// NextNode - The index of the next sibling node or 0.
> -  unsigned NextNode;
> +  unsigned NextNode = 0;
>
>/// ChildNode - The index of the first child node or 0.
> -  unsigned ChildNode;
> +  unsigned ChildNode = 0;
>
>/// ParentNode - The index of the parent node.
> -  unsigned ParentNode;
> -
> -  /// FromType, ToType - The type arguments.
> -  QualType FromType, ToType;
> -
> -  /// FromExpr, ToExpr - The expression arguments.
> -  Expr *FromExpr, *ToExpr;
> -
> -  /// FromNullPtr, ToNullPtr - If the template argument is a nullptr
> -  bool FromNullPtr, ToNullPtr;
> -
> -  /// FromTD, ToTD - The template decl for template template
> -  /// arguments or the type arguments that are templates.
> -  TemplateDecl *FromTD, *ToTD;
> -
> -  /// FromQual, ToQual - Qualifiers for template types.
> -  Qualifiers FromQual, ToQual;
> -
> -  /// FromInt, ToInt - APSInt's for integral arguments.
> -  llvm::APSInt FromInt, ToInt;
> -
> -  /// IsValidFromInt, IsValidToInt - Whether the APSInt's are valid.
> -  bool IsValidFromInt, IsValidToInt;
> +  unsigned ParentNode = 0;
>
> -  /// FromValueDecl, ToValueDecl - Whether the argument is a decl.

r257831 - Refactor template type diffing

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Thu Jan 14 16:56:39 2016
New Revision: 257831

URL: http://llvm.org/viewvc/llvm-project?rev=257831=rev
Log:
Refactor template type diffing

1) Instead of using pairs of From/To* fields, combine fields into a struct
TemplateArgInfo and have two in each DiffNode.
2) Use default initialization in DiffNode so that the constructor shows the
only field that is initialized differently on construction.
3) Use Set and Get functions per each DiffKind to make sure all fields for the
diff is set.  In one case, the Expr fields were not set.
4) Don't print boolean literals for boolean template arguments.  This prevents
printing 'false aka 0'

Only #3 has a functional change, which is reflected in the test change.

Modified:
cfe/trunk/lib/AST/ASTDiagnostic.cpp
cfe/trunk/test/Misc/diag-template-diffing.cpp

Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDiagnostic.cpp?rev=257831=257830=257831=diff
==
--- cfe/trunk/lib/AST/ASTDiagnostic.cpp (original)
+++ cfe/trunk/lib/AST/ASTDiagnostic.cpp Thu Jan 14 16:56:39 2016
@@ -491,82 +491,67 @@ class TemplateDiff {
   /// DiffTree - A tree representation the differences between two types.
   class DiffTree {
   public:
-/// DiffKind - The difference in a DiffNode and which fields are used.
+/// DiffKind - The difference in a DiffNode.  Fields of
+/// TemplateArgumentInfo needed by each difference can be found in the
+/// Set* and Get* functions.
 enum DiffKind {
   /// Incomplete or invalid node.
   Invalid,
-  /// Another level of templates, uses TemplateDecl and Qualifiers
+  /// Another level of templates, requires that
   Template,
-  /// Type difference, uses QualType
+  /// Type difference, all type differences except those falling under
+  /// the Template difference.
   Type,
-  /// Expression difference, uses Expr
+  /// Expression difference, this is only when both arguments are
+  /// expressions.  If one argument is an expression and the other is
+  /// Integer or Declaration, then use that diff type instead.
   Expression,
-  /// Template argument difference, uses TemplateDecl
+  /// Template argument difference
   TemplateTemplate,
-  /// Integer difference, uses APSInt and Expr
+  /// Integer difference
   Integer,
-  /// Declaration difference, uses ValueDecl
+  /// Declaration difference, nullptr arguments are included here
   Declaration
 };
+
   private:
+/// TemplateArgumentInfo - All the information needed to pretty print
+/// a template argument.  See the Set* and Get* functions to see which
+/// fields are used for each DiffKind.
+struct TemplateArgumentInfo {
+  QualType ArgType;
+  Qualifiers Qual;
+  llvm::APSInt Val;
+  bool IsValidInt = false;
+  Expr *ArgExpr = nullptr;
+  TemplateDecl *TD = nullptr;
+  ValueDecl *VD = nullptr;
+  bool NeedAddressOf = false;
+  bool IsNullPtr = false;
+  bool IsDefault = false;
+};
+
 /// DiffNode - The root node stores the original type.  Each child node
 /// stores template arguments of their parents.  For templated types, the
 /// template decl is also stored.
 struct DiffNode {
-  DiffKind Kind;
+  DiffKind Kind = Invalid;
 
   /// NextNode - The index of the next sibling node or 0.
-  unsigned NextNode;
+  unsigned NextNode = 0;
 
   /// ChildNode - The index of the first child node or 0.
-  unsigned ChildNode;
+  unsigned ChildNode = 0;
 
   /// ParentNode - The index of the parent node.
-  unsigned ParentNode;
-
-  /// FromType, ToType - The type arguments.
-  QualType FromType, ToType;
-
-  /// FromExpr, ToExpr - The expression arguments.
-  Expr *FromExpr, *ToExpr;
-
-  /// FromNullPtr, ToNullPtr - If the template argument is a nullptr
-  bool FromNullPtr, ToNullPtr;
-
-  /// FromTD, ToTD - The template decl for template template
-  /// arguments or the type arguments that are templates.
-  TemplateDecl *FromTD, *ToTD;
-
-  /// FromQual, ToQual - Qualifiers for template types.
-  Qualifiers FromQual, ToQual;
-
-  /// FromInt, ToInt - APSInt's for integral arguments.
-  llvm::APSInt FromInt, ToInt;
-
-  /// IsValidFromInt, IsValidToInt - Whether the APSInt's are valid.
-  bool IsValidFromInt, IsValidToInt;
+  unsigned ParentNode = 0;
 
-  /// FromValueDecl, ToValueDecl - Whether the argument is a decl.
-  ValueDecl *FromValueDecl, *ToValueDecl;
-
-  /// FromAddressOf, ToAddressOf - Whether the ValueDecl needs an address 
of
-  /// operator before it.
-  bool FromAddressOf, ToAddressOf;
-
-  /// FromDefault, ToDefault - Whether the argument is a default argument.
-  bool FromDefault, ToDefault;
+  TemplateArgumentInfo