Skip to content

[clang][NFC] Reduce one of JSON dump tests - #195026

Merged
Endilll merged 5 commits into
llvm:mainfrom
Endilll:minimize-ast-dump-test
Apr 30, 2026
Merged

[clang][NFC] Reduce one of JSON dump tests#195026
Endilll merged 5 commits into
llvm:mainfrom
Endilll:minimize-ast-dump-test

Conversation

@Endilll

@Endilll Endilll commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

#125791 introduced strict-pack-match flag for ClassTemplateSpecializationDecl, and covered it with AST dump tests in ast-dump-template.cpp, in both textual and JSON formats. However, JSON test was generated by a script, which made it overspecified. This PR extracts the relevant part of ≈9200 lines of FileCheck directives.

@Endilll Endilll added the clang Clang issues not falling into any other category label Apr 30, 2026
@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

#125791 introduced strict-pack-match flag for ClassTemplateSpecializationDecl, and covered it with AST dump tests in ast-dump-template.cpp, in both textual and JSON formats. However, JSON test was generated by a script, which made it overspecified. This PR extracts the relevant part of ≈9200 lines of FileCheck directives.


Patch is 331.63 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/195026.diff

1 Files Affected:

  • (modified) clang/test/AST/ast-dump-templates.cpp (+13-9339)
diff --git a/clang/test/AST/ast-dump-templates.cpp b/clang/test/AST/ast-dump-templates.cpp
index bd89d3a75486a..f00ec4dba7e21 100644
--- a/clang/test/AST/ast-dump-templates.cpp
+++ b/clang/test/AST/ast-dump-templates.cpp
@@ -128,15 +128,23 @@ void func() {
 }
 
 namespace test7 {
-  template <template<class> class TT> struct A {};
+  template <template<class> class TT> struct AA {};
   template <class...> class B {};
-  template struct A<B>;
+  template struct AA<B>;
 // DUMP-LABEL: NamespaceDecl {{.*}} test7{{$}}
-// DUMP:       ClassTemplateDecl 0x{{.+}} A{{$}}
+// DUMP:       ClassTemplateDecl 0x{{.+}} AA{{$}}
 // DUMP-NEXT:  |-TemplateTemplateParmDecl
 // DUMP-NEXT:  | `-TemplateTypeParmDecl
-// DUMP-NEXT:  |-CXXRecordDecl 0x[[TEST7_PAT:[^ ]+]] {{.+}} struct A definition
-// DUMP:       ClassTemplateSpecializationDecl {{.*}} struct A definition instantiated_from 0x[[TEST7_PAT]] explicit_instantiation_definition strict-pack-match{{$}}
+// DUMP-NEXT:  |-CXXRecordDecl 0x[[TEST7_PAT:[^ ]+]] {{.+}} struct AA definition
+// DUMP:       ClassTemplateSpecializationDecl {{.*}} struct AA definition instantiated_from 0x[[TEST7_PAT]] explicit_instantiation_definition strict-pack-match{{$}}
+
+// JSON:       "name": "test7",
+// JSON:        "kind": "ClassTemplateSpecializationDecl",
+// JSON:        "name": "AA",
+// JSON-NEXT:   "tagUsed": "struct",
+// JSON-NEXT:   "completeDefinition": true,
+// JSON-NEXT:   "strict-pack-match": true,
+// JSON:       "name": "test8",
 } // namespce test7
 
 namespace test8 {
@@ -276,9337 +284,3 @@ namespace TestAbbreviatedTemplateDecls {
 // DUMP: FunctionTemplateDecl {{.*}} <line:[[@LINE-4]]:3, line:[[@LINE-3]]:21> col:8 mixed
 
 } // namespace TestAbbreviatedTemplateDecls
-
-// NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
-
-
-// JSON-NOT: {{^}}Dumping
-// JSON:  "kind": "TranslationUnitDecl",
-// JSON-NEXT:  "loc": {},
-// JSON-NEXT:  "range": {
-// JSON-NEXT:   "begin": {},
-// JSON-NEXT:   "end": {}
-// JSON-NEXT:  },
-// JSON-NEXT:  "inner": [
-// JSON-NEXT:   {
-// JSON-NEXT:    "id": "0x{{.*}}",
-// JSON-NEXT:    "kind": "TypedefDecl",
-// JSON-NEXT:    "loc": {},
-// JSON-NEXT:    "range": {
-// JSON-NEXT:     "begin": {},
-// JSON-NEXT:     "end": {}
-// JSON-NEXT:    },
-// JSON-NEXT:    "isImplicit": true,
-// JSON-NEXT:    "name": "__int128_t",
-// JSON-NEXT:    "type": {
-// JSON-NEXT:     "qualType": "__int128"
-// JSON-NEXT:    },
-// JSON-NEXT:    "inner": [
-// JSON-NEXT:     {
-// JSON-NEXT:      "id": "0x{{.*}}",
-// JSON-NEXT:      "kind": "BuiltinType",
-// JSON-NEXT:      "type": {
-// JSON-NEXT:       "qualType": "__int128"
-// JSON-NEXT:      }
-// JSON-NEXT:     }
-// JSON-NEXT:    ]
-// JSON-NEXT:   },
-// JSON-NEXT:   {
-// JSON-NEXT:    "id": "0x{{.*}}",
-// JSON-NEXT:    "kind": "TypedefDecl",
-// JSON-NEXT:    "loc": {},
-// JSON-NEXT:    "range": {
-// JSON-NEXT:     "begin": {},
-// JSON-NEXT:     "end": {}
-// JSON-NEXT:    },
-// JSON-NEXT:    "isImplicit": true,
-// JSON-NEXT:    "name": "__uint128_t",
-// JSON-NEXT:    "type": {
-// JSON-NEXT:     "qualType": "unsigned __int128"
-// JSON-NEXT:    },
-// JSON-NEXT:    "inner": [
-// JSON-NEXT:     {
-// JSON-NEXT:      "id": "0x{{.*}}",
-// JSON-NEXT:      "kind": "BuiltinType",
-// JSON-NEXT:      "type": {
-// JSON-NEXT:       "qualType": "unsigned __int128"
-// JSON-NEXT:      }
-// JSON-NEXT:     }
-// JSON-NEXT:    ]
-// JSON-NEXT:   },
-// JSON-NEXT:   {
-// JSON-NEXT:    "id": "0x{{.*}}",
-// JSON-NEXT:    "kind": "TypedefDecl",
-// JSON-NEXT:    "loc": {},
-// JSON-NEXT:    "range": {
-// JSON-NEXT:     "begin": {},
-// JSON-NEXT:     "end": {}
-// JSON-NEXT:    },
-// JSON-NEXT:    "isImplicit": true,
-// JSON-NEXT:    "name": "__NSConstantString",
-// JSON-NEXT:    "type": {
-// JSON-NEXT:     "qualType": "__NSConstantString_tag"
-// JSON-NEXT:    },
-// JSON-NEXT:    "inner": [
-// JSON-NEXT:     {
-// JSON-NEXT:      "id": "0x{{.*}}",
-// JSON-NEXT:      "kind": "RecordType",
-// JSON-NEXT:      "type": {
-// JSON-NEXT:       "qualType": "__NSConstantString_tag"
-// JSON-NEXT:      },
-// JSON-NEXT:      "decl": {
-// JSON-NEXT:       "id": "0x{{.*}}",
-// JSON-NEXT:       "kind": "CXXRecordDecl",
-// JSON-NEXT:       "name": "__NSConstantString_tag"
-// JSON-NEXT:      }
-// JSON-NEXT:     }
-// JSON-NEXT:    ]
-// JSON-NEXT:   },
-// JSON-NEXT:   {
-// JSON-NEXT:    "id": "0x{{.*}}",
-// JSON-NEXT:    "kind": "TypedefDecl",
-// JSON-NEXT:    "loc": {},
-// JSON-NEXT:    "range": {
-// JSON-NEXT:     "begin": {},
-// JSON-NEXT:     "end": {}
-// JSON-NEXT:    },
-// JSON-NEXT:    "isImplicit": true,
-// JSON-NEXT:    "name": "__builtin_ms_va_list",
-// JSON-NEXT:    "type": {
-// JSON-NEXT:     "qualType": "char *"
-// JSON-NEXT:    },
-// JSON-NEXT:    "inner": [
-// JSON-NEXT:     {
-// JSON-NEXT:      "id": "0x{{.*}}",
-// JSON-NEXT:      "kind": "PointerType",
-// JSON-NEXT:      "type": {
-// JSON-NEXT:       "qualType": "char *"
-// JSON-NEXT:      },
-// JSON-NEXT:      "inner": [
-// JSON-NEXT:       {
-// JSON-NEXT:        "id": "0x{{.*}}",
-// JSON-NEXT:        "kind": "BuiltinType",
-// JSON-NEXT:        "type": {
-// JSON-NEXT:         "qualType": "char"
-// JSON-NEXT:        }
-// JSON-NEXT:       }
-// JSON-NEXT:      ]
-// JSON-NEXT:     }
-// JSON-NEXT:    ]
-// JSON-NEXT:   },
-// JSON-NEXT:   {
-// JSON-NEXT:    "id": "0x{{.*}}",
-// JSON-NEXT:    "kind": "TypedefDecl",
-// JSON-NEXT:    "loc": {},
-// JSON-NEXT:    "range": {
-// JSON-NEXT:     "begin": {},
-// JSON-NEXT:     "end": {}
-// JSON-NEXT:    },
-// JSON-NEXT:    "isImplicit": true,
-// JSON-NEXT:    "name": "__builtin_va_list",
-// JSON-NEXT:    "type": {
-// JSON-NEXT:     "qualType": "__va_list_tag[1]"
-// JSON-NEXT:    },
-// JSON-NEXT:    "inner": [
-// JSON-NEXT:     {
-// JSON-NEXT:      "id": "0x{{.*}}",
-// JSON-NEXT:      "kind": "ConstantArrayType",
-// JSON-NEXT:      "type": {
-// JSON-NEXT:       "qualType": "__va_list_tag[1]"
-// JSON-NEXT:      },
-// JSON-NEXT:      "size": 1,
-// JSON-NEXT:      "inner": [
-// JSON-NEXT:       {
-// JSON-NEXT:        "id": "0x{{.*}}",
-// JSON-NEXT:        "kind": "RecordType",
-// JSON-NEXT:        "type": {
-// JSON-NEXT:         "qualType": "__va_list_tag"
-// JSON-NEXT:        },
-// JSON-NEXT:        "decl": {
-// JSON-NEXT:         "id": "0x{{.*}}",
-// JSON-NEXT:         "kind": "CXXRecordDecl",
-// JSON-NEXT:         "name": "__va_list_tag"
-// JSON-NEXT:        }
-// JSON-NEXT:       }
-// JSON-NEXT:      ]
-// JSON-NEXT:     }
-// JSON-NEXT:    ]
-// JSON-NEXT:   },
-// JSON-NEXT:   {
-// JSON-NEXT:    "id": "0x{{.*}}",
-// JSON-NEXT:    "kind": "ClassTemplateDecl",
-// JSON-NEXT:    "loc": {
-// JSON-NEXT:     "offset": 812,
-// JSON-NEXT:     "file": "{{.*}}",
-// JSON-NEXT:     "line": 15,
-// JSON-NEXT:     "col": 8,
-// JSON-NEXT:     "tokLen": 3
-// JSON-NEXT:    },
-// JSON-NEXT:    "range": {
-// JSON-NEXT:     "begin": {
-// JSON-NEXT:      "offset": 765,
-// JSON-NEXT:      "line": 14,
-// JSON-NEXT:      "col": 1,
-// JSON-NEXT:      "tokLen": 8
-// JSON-NEXT:     },
-// JSON-NEXT:     "end": {
-// JSON-NEXT:      "offset": 879,
-// JSON-NEXT:      "line": 19,
-// JSON-NEXT:      "col": 1,
-// JSON-NEXT:      "tokLen": 1
-// JSON-NEXT:     }
-// JSON-NEXT:    },
-// JSON-NEXT:    "isReferenced": true,
-// JSON-NEXT:    "name": "foo",
-// JSON-NEXT:    "inner": [
-// JSON-NEXT:     {
-// JSON-NEXT:      "id": "0x{{.*}}",
-// JSON-NEXT:      "kind": "NonTypeTemplateParmDecl",
-// JSON-NEXT:      "loc": {
-// JSON-NEXT:       "offset": 779,
-// JSON-NEXT:       "line": 14,
-// JSON-NEXT:       "col": 15,
-// JSON-NEXT:       "tokLen": 1
-// JSON-NEXT:      },
-// JSON-NEXT:      "range": {
-// JSON-NEXT:       "begin": {
-// JSON-NEXT:        "offset": 775,
-// JSON-NEXT:        "col": 11,
-// JSON-NEXT:        "tokLen": 3
-// JSON-NEXT:       },
-// JSON-NEXT:       "end": {
-// JSON-NEXT:        "offset": 779,
-// JSON-NEXT:        "col": 15,
-// JSON-NEXT:        "tokLen": 1
-// JSON-NEXT:       }
-// JSON-NEXT:      },
-// JSON-NEXT:      "isReferenced": true,
-// JSON-NEXT:      "name": "X",
-// JSON-NEXT:      "type": {
-// JSON-NEXT:       "qualType": "int"
-// JSON-NEXT:      },
-// JSON-NEXT:      "depth": 0,
-// JSON-NEXT:      "index": 0
-// JSON-NEXT:     },
-// JSON-NEXT:     {
-// JSON-NEXT:      "id": "0x{{.*}}",
-// JSON-NEXT:      "kind": "TemplateTypeParmDecl",
-// JSON-NEXT:      "loc": {
-// JSON-NEXT:       "offset": 791,
-// JSON-NEXT:       "col": 27,
-// JSON-NEXT:       "tokLen": 1
-// JSON-NEXT:      },
-// JSON-NEXT:      "range": {
-// JSON-NEXT:       "begin": {
-// JSON-NEXT:        "offset": 782,
-// JSON-NEXT:        "col": 18,
-// JSON-NEXT:        "tokLen": 8
-// JSON-NEXT:       },
-// JSON-NEXT:       "end": {
-// JSON-NEXT:        "offset": 791,
-// JSON-NEXT:        "col": 27,
-// JSON-NEXT:        "tokLen": 1
-// JSON-NEXT:       }
-// JSON-NEXT:      },
-// JSON-NEXT:      "isReferenced": true,
-// JSON-NEXT:      "name": "Y",
-// JSON-NEXT:      "tagUsed": "typename",
-// JSON-NEXT:      "depth": 0,
-// JSON-NEXT:      "index": 1
-// JSON-NEXT:     },
-// JSON-NEXT:     {
-// JSON-NEXT:      "id": "0x{{.*}}",
-// JSON-NEXT:      "kind": "NonTypeTemplateParmDecl",
-// JSON-NEXT:      "loc": {
-// JSON-NEXT:       "offset": 798,
-// JSON-NEXT:       "col": 34,
-// JSON-NEXT:       "tokLen": 1
-// JSON-NEXT:      },
-// JSON-NEXT:      "range": {
-// JSON-NEXT:       "begin": {
-// JSON-NEXT:        "offset": 794,
-// JSON-NEXT:        "col": 30,
-// JSON-NEXT:        "tokLen": 3
-// JSON-NEXT:       },
-// JSON-NEXT:       "end": {
-// JSON-NEXT:        "offset": 802,
-// JSON-NEXT:        "col": 38,
-// JSON-NEXT:        "tokLen": 1
-// JSON-NEXT:       }
-// JSON-NEXT:      },
-// JSON-NEXT:      "isReferenced": true,
-// JSON-NEXT:      "name": "Z",
-// JSON-NEXT:      "type": {
-// JSON-NEXT:       "qualType": "int"
-// JSON-NEXT:      },
-// JSON-NEXT:      "depth": 0,
-// JSON-NEXT:      "index": 2,
-// JSON-NEXT:      "defaultArg": {
-// JSON-NEXT:       "kind": "TemplateArgument",
-// JSON-NEXT:       "isExpr": true
-// JSON-NEXT:      },
-// JSON-NEXT:      "inner": [
-// JSON-NEXT:       {
-// JSON-NEXT:        "kind": "TemplateArgument",
-// JSON-NEXT:        "range": {
-// JSON-NEXT:         "begin": {
-// JSON-NEXT:          "offset": 802,
-// JSON-NEXT:          "col": 38,
-// JSON-NEXT:          "tokLen": 1
-// JSON-NEXT:         },
-// JSON-NEXT:         "end": {
-// JSON-NEXT:          "offset": 802,
-// JSON-NEXT:          "col": 38,
-// JSON-NEXT:          "tokLen": 1
-// JSON-NEXT:         }
-// JSON-NEXT:        },
-// JSON-NEXT:        "isExpr": true,
-// JSON-NEXT:        "inner": [
-// JSON-NEXT:         {
-// JSON-NEXT:          "id": "0x{{.*}}",
-// JSON-NEXT:          "kind": "IntegerLiteral",
-// JSON-NEXT:          "range": {
-// JSON-NEXT:           "begin": {
-// JSON-NEXT:            "offset": 802,
-// JSON-NEXT:            "col": 38,
-// JSON-NEXT:            "tokLen": 1
-// JSON-NEXT:           },
-// JSON-NEXT:           "end": {
-// JSON-NEXT:            "offset": 802,
-// JSON-NEXT:            "col": 38,
-// JSON-NEXT:            "tokLen": 1
-// JSON-NEXT:           }
-// JSON-NEXT:          },
-// JSON-NEXT:          "type": {
-// JSON-NEXT:           "qualType": "int"
-// JSON-NEXT:          },
-// JSON-NEXT:          "valueCategory": "prvalue",
-// JSON-NEXT:          "value": "5"
-// JSON-NEXT:         }
-// JSON-NEXT:        ]
-// JSON-NEXT:       }
-// JSON-NEXT:      ]
-// JSON-NEXT:     },
-// JSON-NEXT:     {
-// JSON-NEXT:      "id": "0x{{.*}}",
-// JSON-NEXT:      "kind": "CXXRecordDecl",
-// JSON-NEXT:      "loc": {
-// JSON-NEXT:       "offset": 812,
-// JSON-NEXT:       "line": 15,
-// JSON-NEXT:       "col": 8,
-// JSON-NEXT:       "tokLen": 3
-// JSON-NEXT:      },
-// JSON-NEXT:      "range": {
-// JSON-NEXT:       "begin": {
-// JSON-NEXT:        "offset": 805,
-// JSON-NEXT:        "col": 1,
-// JSON-NEXT:        "tokLen": 6
-// JSON-NEXT:       },
-// JSON-NEXT:       "end": {
-// JSON-NEXT:        "offset": 879,
-// JSON-NEXT:        "line": 19,
-// JSON-NEXT:        "col": 1,
-// JSON-NEXT:        "tokLen": 1
-// JSON-NEXT:       }
-// JSON-NEXT:      },
-// JSON-NEXT:      "name": "foo",
-// JSON-NEXT:      "tagUsed": "struct",
-// JSON-NEXT:      "completeDefinition": true,
-// JSON-NEXT:      "definitionData": {
-// JSON-NEXT:       "canConstDefaultInit": true,
-// JSON-NEXT:       "copyAssign": {
-// JSON-NEXT:        "hasConstParam": true,
-// JSON-NEXT:        "implicitHasConstParam": true,
-// JSON-NEXT:        "needsImplicit": true,
-// JSON-NEXT:        "simple": true,
-// JSON-NEXT:        "trivial": true
-// JSON-NEXT:       },
-// JSON-NEXT:       "copyCtor": {
-// JSON-NEXT:        "hasConstParam": true,
-// JSON-NEXT:        "implicitHasConstParam": true,
-// JSON-NEXT:        "needsImplicit": true,
-// JSON-NEXT:        "simple": true,
-// JSON-NEXT:        "trivial": true
-// JSON-NEXT:       },
-// JSON-NEXT:       "defaultCtor": {
-// JSON-NEXT:        "defaultedIsConstexpr": true,
-// JSON-NEXT:        "exists": true,
-// JSON-NEXT:        "nonTrivial": true,
-// JSON-NEXT:        "userProvided": true
-// JSON-NEXT:       },
-// JSON-NEXT:       "dtor": {
-// JSON-NEXT:        "irrelevant": true,
-// JSON-NEXT:        "needsImplicit": true,
-// JSON-NEXT:        "simple": true,
-// JSON-NEXT:        "trivial": true
-// JSON-NEXT:       },
-// JSON-NEXT:       "hasUserDeclaredConstructor": true,
-// JSON-NEXT:       "isStandardLayout": true,
-// JSON-NEXT:       "isTriviallyCopyable": true,
-// JSON-NEXT:       "moveAssign": {
-// JSON-NEXT:        "exists": true,
-// JSON-NEXT:        "needsImplicit": true,
-// JSON-NEXT:        "simple": true,
-// JSON-NEXT:        "trivial": true
-// JSON-NEXT:       },
-// JSON-NEXT:       "moveCtor": {
-// JSON-NEXT:        "exists": true,
-// JSON-NEXT:        "needsImplicit": true,
-// JSON-NEXT:        "simple": true,
-// JSON-NEXT:        "trivial": true
-// JSON-NEXT:       }
-// JSON-NEXT:      },
-// JSON-NEXT:      "inner": [
-// JSON-NEXT:       {
-// JSON-NEXT:        "id": "0x{{.*}}",
-// JSON-NEXT:        "kind": "CXXRecordDecl",
-// JSON-NEXT:        "loc": {
-// JSON-NEXT:         "offset": 812,
-// JSON-NEXT:         "line": 15,
-// JSON-NEXT:         "col": 8,
-// JSON-NEXT:         "tokLen": 3
-// JSON-NEXT:        },
-// JSON-NEXT:        "range": {
-// JSON-NEXT:         "begin": {
-// JSON-NEXT:          "offset": 805,
-// JSON-NEXT:          "col": 1,
-// JSON-NEXT:          "tokLen": 6
-// JSON-NEXT:         },
-// JSON-NEXT:         "end": {
-// JSON-NEXT:          "offset": 812,
-// JSON-NEXT:          "col": 8,
-// JSON-NEXT:          "tokLen": 3
-// JSON-NEXT:         }
-// JSON-NEXT:        },
-// JSON-NEXT:        "isImplicit": true,
-// JSON-NEXT:        "name": "foo",
-// JSON-NEXT:        "tagUsed": "struct"
-// JSON-NEXT:       },
-// JSON-NEXT:       {
-// JSON-NEXT:        "id": "0x{{.*}}",
-// JSON-NEXT:        "kind": "FieldDecl",
-// JSON-NEXT:        "loc": {
-// JSON-NEXT:         "offset": 824,
-// JSON-NEXT:         "line": 16,
-// JSON-NEXT:         "col": 7,
-// JSON-NEXT:         "tokLen": 8
-// JSON-NEXT:        },
-// JSON-NEXT:        "range": {
-// JSON-NEXT:         "begin": {
-// JSON-NEXT:          "offset": 820,
-// JSON-NEXT:          "col": 3,
-// JSON-NEXT:          "tokLen": 3
-// JSON-NEXT:         },
-// JSON-NEXT:         "end": {
-// JSON-NEXT:          "offset": 824,
-// JSON-NEXT:          "col": 7,
-// JSON-NEXT:          "tokLen": 8
-// JSON-NEXT:         }
-// JSON-NEXT:        },
-// JSON-NEXT:        "name": "constant",
-// JSON-NEXT:        "type": {
-// JSON-NEXT:         "qualType": "int"
-// JSON-NEXT:        }
-// JSON-NEXT:       },
-// JSON-NEXT:       {
-// JSON-NEXT:        "id": "0x{{.*}}",
-// JSON-NEXT:        "kind": "CXXConstructorDecl",
-// JSON-NEXT:        "loc": {
-// JSON-NEXT:         "offset": 836,
-// JSON-NEXT:         "line": 17,
-// JSON-NEXT:         "col": 3,
-// JSON-NEXT:         "tokLen": 3
-// JSON-NEXT:        },
-// JSON-NEXT:        "range": {
-// JSON-NEXT:         "begin": {
-// JSON-NEXT:          "offset": 836,
-// JSON-NEXT:          "col": 3,
-// JSON-NEXT:          "tokLen": 3
-// JSON-NEXT:         },
-// JSON-NEXT:         "end": {
-// JSON-NEXT:          "offset": 843,
-// JSON-NEXT:          "col": 10,
-// JSON-NEXT:          "tokLen": 1
-// JSON-NEXT:         }
-// JSON-NEXT:        },
-// JSON-NEXT:        "name": "foo<X, Y, Z>",
-// JSON-NEXT:        "type": {
-// JSON-NEXT:         "qualType": "void ()"
-// JSON-NEXT:        },
-// JSON-NEXT:        "inner": [
-// JSON-NEXT:         {
-// JSON-NEXT:          "id": "0x{{.*}}",
-// JSON-NEXT:          "kind": "CompoundStmt",
-// JSON-NEXT:          "range": {
-// JSON-NEXT:           "begin": {
-// JSON-NEXT:            "offset": 842,
-// JSON-NEXT:            "col": 9,
-// JSON-NEXT:            "tokLen": 1
-// JSON-NEXT:           },
-// JSON-NEXT:           "end": {
-// JSON-NEXT:            "offset": 843,
-// JSON-NEXT:            "col": 10,
-// JSON-NEXT:            "tokLen": 1
-// JSON-NEXT:           }
-// JSON-NEXT:          }
-// JSON-NEXT:         }
-// JSON-NEXT:        ]
-// JSON-NEXT:       },
-// JSON-NEXT:       {
-// JSON-NEXT:        "id": "0x{{.*}}",
-// JSON-NEXT:        "kind": "CXXMethodDecl",
-// JSON-NEXT:        "loc": {
-// JSON-NEXT:         "offset": 849,
-// JSON-NEXT:         "line": 18,
-// JSON-NEXT:         "col": 5,
-// JSON-NEXT:         "tokLen": 6
-// JSON-NEXT:        },
-// JSON-NEXT:        "range": {
-// JSON-NEXT:         "begin": {
-// JSON-NEXT:          "offset": 847,
-// JSON-NEXT:          "col": 3,
-// JSON-NEXT:          "tokLen": 1
-// JSON-NEXT:         },
-// JSON-NEXT:         "end": {
-// JSON-NEXT:          "offset": 877,
-// JSON-NEXT:          "col": 33,
-// JSON-NEXT:          "tokLen": 1
-// JSON-NEXT:         }
-// JSON-NEXT:        },
-// JSON-NEXT:        "name": "getSum",
-// JSON-NEXT:        "type": {
-// JSON-NEXT:         "qualType": "Y ()"
-// JSON-NEXT:        },
-// JSON-NEXT:        "inner": [
-// JSON-NEXT:         {
-// JSON-NEXT:          "id": "0x{{.*}}",
-// JSON-NEXT:          "kind": "CompoundStmt",
-// JSON-NEXT:          "range": {
-// JSON-NEXT:           "begin": {
-// JSON-NEXT:            "offset": 858,
-// JSON-NEXT:            "col": 14,
-// JSON-NEXT:            "tokLen": 1
-// JSON-NEXT:           },
-// JSON-NEXT:           "end": {
-// JSON-NEXT:            "offset": 877,
-// JSON-NEXT:            "col": 33,
-// JSON-NEXT:            "tokLen": 1
-// JSON-NEXT:           }
-// JSON-NEXT:          },
-// JSON-NEXT:          "inner": [
-// JSON-NEXT:           {
-// JSON-NEXT:            "id": "0x{{.*}}",
-// JSON-NEXT:            "kind": "ReturnStmt",
-// JSON-NEXT:            "range": {
-// JSON-NEXT:             "begin": {
-// JSON-NEXT:              "offset": 860,
-// JSON-NEXT:              "col": 16,
-// JSON-NEXT:              "tokLen": 6
-// JSON-NEXT:             },
-// JSON-NEXT:             "end": {
-// JSON-NEXT:              "offset": 874,
-// JSON-NEXT:              "col": 30,
-// JSON-NEXT:              "tokLen": 1
-// JSON-NEXT:             }
-// JSON-NEXT:            },
-// JSON-NEXT:            "inner": [
-// JSON-NEXT:             {
-// JSON-NEXT:              "id": "0x{{.*}}",
-// JSON-NEXT:              "kind": "CXXUnresolvedConstructExpr",
-// JSON-NEXT:              "range": {
-// JSON-NEXT:               "begin": {
-// JSON-NEXT:                "offset": 867,
-// JSON-NEXT:                "col": 23,
-// JSON-NEXT:                "tokLen": 1
-// JSON-NEXT:               },
-// JSON-NEXT:               "end": {
-// JSON-NEXT:                "offset": 874,
-// JSON-NEXT:                "col": 30,
-// JSON-NEXT:                "tokLen": 1
-// JSON-NEXT:               }
-// JSON-NEXT:              },
-// JSON-NEXT:              "type": {
-// JSON-NEXT:               "qualType": "Y"
-// JSON-NEXT:              },
-// JSON-NEXT:              "valueCategory": "prvalue",
-// JSON-NEXT:              "inner": [
-// JSON-NEXT:               {
-// JSON-NEXT:                "id": "0x{{.*}}",
-// JSON-NEXT:                "kind": "BinaryOperator",
-// JSON-NEXT:                "range": {
-// JSON-NEXT:                 "begin": {
-// JSON-NEXT:                  "offset": 869,
-/...
[truncated]

@cor3ntin cor3ntin changed the title [clang] Reduce one of JSON dump tests [clang][NFC] Reduce one of JSON dump tests Apr 30, 2026

@mizvekov mizvekov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of this json test was not just these flags.

We had low coverage of some template stuff in json test, and this was a drive by improvement.

For example, since this test was added, I have relied on it to test multiple other PRs.

@AaronBallman

Copy link
Copy Markdown
Contributor

The whole point of this json test was not just these flags.

We had low coverage of some template stuff in json test, and this was a drive by improvement.

For example, since this test was added, I have relied on it to test multiple other PRs.

FWIW, we should be using these kinds of tests only as an absolute last resort; these tests are some of the most frustrating to maintain long-term. Personally, I think the test is in much better shape with these changes and I would like to see it proceed; if it was testing multiple things, then those can be split out into their own small tests as well, but keeping them in one massive 9kb test file is not maintainable IMO. (I would love to someday see all of the AST dumping tests eventually go away except for ones intended to test crashing regression fixes or similar.)

@Endilll

Endilll commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

The whole point of this json test was not just these flags.

Unlike CHECK lines for the textual AST dump, the sheer size of the JSON dump obscures what exactly is being tested, whatever that is.

@mizvekov

Copy link
Copy Markdown
Contributor

So this PR replaces one automatically generated large test, with a small manually maintained one. That doesn't seem like a maintainability improvement to me.

Also, the change looks to maintain only things considered important when looking at the original PR which introduced it, but fails to look at other subsequent PRs which introduced other changes which relied on test changes to this file.

@mizvekov

Copy link
Copy Markdown
Contributor

Also, why only this test? There are multiple other large automatically generated AST JSON dump tests.

However, this file contains some source code patterns which are unique relative to these other tests.

I think this reduces our ability to maintain the JSON dumping infrastructure.

@AaronBallman

Copy link
Copy Markdown
Contributor

So this PR replaces one automatically generated large test, with a small manually maintained one. That doesn't seem like a maintainability improvement to me.

It does to me. I can look at this test and understand what it does; I have absolutely no idea what the previous form was attempting to test.

Also, the change looks to maintain only things considered important when looking at the original PR which introduced it, but fails to look at other subsequent PRs which introduced other changes which relied on test changes to this file.

I think that should be addressed before we land.

Also, why only this test? There are multiple other large automatically generated AST JSON dump tests.

Yes, for example the OpenMP generated tests are a frequent source of frustration when changing dumping behavior. Other tests are doing bad things too and should be updated, but it doesn't need to be part of this PR.

I think this reduces our ability to maintain the JSON dumping infrastructure.

AST dumping is like AST printing; it's always best effort and is a debugging or research aid. The testing needs for it should not outweigh the benefits we get from it, and AST dumping tests have gotten sufficiently expensive to maintain that we sometimes don't update the dumpers because of how painful it is to maintain the over-tested files which used a script. (Frankly, I find those AST dumping scripts to cause more harm than benefit in terms of the end result; if we can improve the scripts, that would be really great)

@Endilll

Endilll commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

So this PR replaces one automatically generated large test, with a small manually maintained one. That doesn't seem like a maintainability improvement to me.

By this logic, we should throw away all our manually-written tests, and replace them with the output of test-generating scripts, because running a script is much easier than thinking what exactly needs to be tested.

It's easier to maintain, because it's easier to see what is important. For instance, it doesn't capture absolutely useless details like byte offsets or predefined __int128.

I think this reduces our ability to maintain the JSON dumping infrastructure.

Our ability to maintain anything is reduced by huge tests in the same way big reproducers get in the way of understanding what's going on in any given issue.

Also, why only this test? There are multiple other large automatically generated AST JSON dump tests.

Because I'm working on another patch that modifies textual AST dump (#194600), and this was the only test I had to touch that captured byte offsets.

However, this file contains some source code patterns which are unique relative to these other tests.

I definitely captured one of those patterns. I can add more, but I'm at a loss what exactly is useful in that 9k lines dump. I do not believe we need all of that, as we're only matching ≈100 lines out of ≈550 lines of textual AST dump.

@Endilll

Endilll commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

Yes, for example the OpenMP generated tests are a frequent source of frustration when changing dumping behavior.

I can confirm that OpenMP tests were a significant pain for me, but the test I'm changing here is in an entirely different league in terms of both size and signal-to-noise ratio.

@mizvekov

Copy link
Copy Markdown
Contributor

I think we have really opposite views about what's difficult to maintain here:

To me the problem about the manually crafted textual AST dump tests is that:

  • They are really tedious to update.
  • They often don't capture enough surrounding context to be useful or readable, and don't avoid false matches on other surrounding tests.

The JSON dump tests avoid those problems, and you really only run a script to update them, and then check the differences on the diff viewer. I'd rather be doing that than struggling with the keyboard to update these tests.

Regarding the random test update noise, the JSON tests can easily be improved by removing the line / byte-offsets from most tests, and that can be done simply changing the test generator script.

@AaronBallman

Copy link
Copy Markdown
Contributor

I think we have really opposite views about what's difficult to maintain here:

To me the problem about the manually crafted textual AST dump tests is that:

* They are really tedious to update.

Having maintained these files for years, I find the manually crafted AST dump tests to be significantly easier to update. They often require fewer updates when the dumper changes because they're not as sensitive to AST dumping changes due to being more targeted.

* They often don't capture enough surrounding context to be useful or readable, and don't avoid false matches on other surrounding tests.

This is the opposite of my problem with the over-testing of AST dumps; those often have so much useless context that it becomes impossible to see what's being tested. Neither situation is ideal, but I find "too much context" to be worse than "too little context" on balance.

The JSON dump tests avoid those problems, and you really only run a script to update them, and then check the differences on the diff viewer. I'd rather be doing that than struggling with the keyboard to update these tests.

I think it introduces more problems than it avoids unless you filter the tests far better. As an example, you are testing that we correctly match against __builtin_ms_va_list which has absolutely nothing to do with anything in the test file. This kind of needless over-testing makes the test harder to read, slower to run, and harder to maintain.

Regarding the random test update noise, the JSON tests can easily be improved by removing the line / byte-offsets from most tests, and that can be done simply changing the test generator script.

That scratches the surface of the problems of using the test generator script; there's still the issue of it dumping nodes entirely unrelated to what's being tested. We have some amount of filtering for that, but not very much. Perhaps the script could be updated in some way to work with AST matchers so you can filter by more than just identifiers, but all of this is exploratory and I continue to believe this PR is good incremental progress in the direction we want to go (modulo any accidentally dropped test coverage, but it's still unclear to me what exactly that is).

@mizvekov

Copy link
Copy Markdown
Contributor

One small example of what I mean is: git show ba9d1c41c41d568a798e0a8c38a89d294647c28d -- clang/test/AST/ast-dump-templates.cpp

These kinds of updates to the textual AST dump are really annoying to perform manually. And this is even one of the easy ones, where there is enough context to actually understand what's changing. Most textual AST dump tests lack CHECK-NEXT lines.

I think the point of these tests is to be exhaustive. I don't expect to just be able to read the test files under clang/test/AST in isolation and see a small set of things being tested. The value here is seeing what changed under those tests for a given commit.

@AaronBallman AaronBallman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is dropping test coverage, @mizvekov please identify explicitly which coverage is relevant so that we can re-add it (perhaps in a separate test file if more appropriate). The discussion about testing theory doesn't seem to be converging but also is orthogonal; as a maintainer, I don't think the current test is in a reasonable state and would prefer to make progress on it over debating whether AST dump tests should or should not be exhaustive, scripted, etc. So barring specific issues with dropped coverage, the changes LGTM as-is.

@mizvekov

Copy link
Copy Markdown
Contributor

All of these commits have changes to the JSON dump part of this file which are relevant to the commit in question:
fb02433
91ebd4a
696946f
398f6c2
ba9d1c4
0e33195
eeada0d
91cdd35
13b55ad
dc17429
89cfeeb
14f7bd6

@mizvekov

Copy link
Copy Markdown
Contributor

A simpler way to put it, is that if JSON dumper has equal footing to the Text dumper, then it would only be reasonable that the each test case tests both equally.

That makes life twice as hard when updating these tests, but at least that would be consistent.

@shafik

shafik commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

I like this change as well. I don't think having a massive test is helpful. We should have a set of smaller tests that test specific fixes and or features. Maybe I could get behind one large test that is broken into pieces w/ extensive comments in between each section explaining what it is testing in each section but large files are just difficult to maintain.

@AaronBallman

Copy link
Copy Markdown
Contributor

All of these commits have changes to the JSON dump part of this file which are relevant to the commit in question: fb02433 91ebd4a 696946f 398f6c2 ba9d1c4 0e33195 eeada0d 91cdd35 13b55ad dc17429 89cfeeb 14f7bd6

I'm not going to spend much more time on review here because it's getting to be extractive at this point, but some of these commits have me worried that AST dumping tests are being used instead of more appropriate testing mechanisms. For example, commit eeada0d should not be using AST dumping tests, it can use diagnostics to test the source range information. We have -fdiagnostics-print-source-range-info specifically to help test source ranges. Commit 89cfeeb added only AST dumping tests despite changing structural equivalence code that could have been tested through other means like serialization related diagnostics. Commit 91cdd35 changed 539 files and is not really an example of the kinds of PRs we want generally; it's not easy to tell how the AST dumping changes relate to the PR or its changes.

tl;dr: as lead maintainer, (in general) I want less AST dumping tests to be added in tree, for JSON or otherwise. They have become too much of a maintenance burden for something which is intended to be used as a debugging and research aid. Having regression tests to ensure we don't crash is a reasonable thing for us to test, but to my mind, they are not to be used for exhaustive testing unless there's a strong justification for why the testing shouldn't use less fragile tools. If someone wants to invest effort in the scripts to make them more usable, I think that's worthwhile, but as the scripts are today, I'd be fine removing them entirely from the tree so others don't mistakenly use them.

@Endilll Endilll left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simpler way to put it, is that if JSON dumper has equal footing to the Text dumper, then it would only be reasonable that the each test case tests both equally.

Thank you for mentioning that, because I'd suggest this otherwise. PR is now updated with more matching of JSON dump, mirroring textual dump tests.

I left comments for two FIXMEs where I think JSON dump is missing information compared to textual dump. Fixing them is out of scope, but if @mizvekov can confirm whether they are real issues, I'd appreciate that.

Comment thread clang/test/AST/ast-dump-templates.cpp Outdated
Comment thread clang/test/AST/ast-dump-templates.cpp Outdated

@mizvekov mizvekov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this now feels consistent and I am happy we are not dropping anything.

The problem remains that it's painful to update these tests when they do change. This is the same pain as updating the Text dumps, and now that workload is doubled at least.

We really ought to have an automated way to update these, but that can be in scope for another PR.

Comment thread clang/test/AST/ast-dump-templates.cpp Outdated
Comment thread clang/test/AST/ast-dump-templates.cpp Outdated

@mizvekov mizvekov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Endilll
Endilll enabled auto-merge (squash) April 30, 2026 18:47
@Endilll
Endilll merged commit f0b2a04 into llvm:main Apr 30, 2026
9 of 10 checks passed
enferex pushed a commit to enferex/llvm-project that referenced this pull request May 5, 2026
llvm#125791 introduced `strict-pack-match` flag for
`ClassTemplateSpecializationDecl`, and covered it with AST dump tests in
`ast-dump-template.cpp`, in both textual and JSON formats. However, JSON
test was generated by a script, which made it overspecified. This PR
extracts the relevant part of ≈9200 lines of FileCheck directives.
moar55 pushed a commit to moar55/llvm-project that referenced this pull request May 12, 2026
llvm#125791 introduced `strict-pack-match` flag for
`ClassTemplateSpecializationDecl`, and covered it with AST dump tests in
`ast-dump-template.cpp`, in both textual and JSON formats. However, JSON
test was generated by a script, which made it overspecified. This PR
extracts the relevant part of ≈9200 lines of FileCheck directives.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants