This is an archive of the discontinued LLVM Phabricator instance.

Differential D146784

[clang] Test for AST Dumping of the template variables
ClosedPublic

Authored by tomasz-kaminski-sonarsource on Mar 23 2023, 11:45 PM.

Details

Reviewers
erichkeane
aaron.ballman
Commits
rG4a9dc94d336a: [clang] Test for AST Dumping of the template variables
Summary

They illustrate unstable behavior of the https://reviews.llvm.org/D139705 after serialization.
(#61680 <https://github.com/llvm/llvm-project/issues/61680>).

Diff Detail

Event Timeline

tomasz-kaminski-sonarsource created this revision.Mar 23 2023, 11:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2023, 11:45 PM
tomasz-kaminski-sonarsource requested review of this revision.Mar 23 2023, 11:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2023, 11:45 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tomasz-kaminski-sonarsource added a reviewer: erichkeane.Mar 23 2023, 11:46 PM

I do not plan to address the behavior in case of a variable without initializer. Creating separate PR to illustrate that is not a regression from my change.

tomasz-kaminski-sonarsource mentioned this in D139705: [clang] fix zero-initialization fix-it for variable template.Mar 23 2023, 11:48 PM
tomasz-kaminski-sonarsource added a child revision: D146733: [clang] source range of variable template specialization should include initializer.
tomasz-kaminski-sonarsource updated this revision to Diff 507973.Mar 23 2023, 11:54 PM

Merged indidictal check lines

tomasz-kaminski-sonarsource added a reviewer: aaron.ballman.Mar 24 2023, 12:05 AM
erichkeane added inline comments.Mar 24 2023, 6:21 AM
clang/test/AST/ast-dump-template-decls.cpp
193

Hmm... thats curious. We shouldn't commit this unless there is a 'fixme' on it showing we don't really mean it, but in reality, this just deserves a Github bug.

tomasz-kaminski-sonarsource updated this revision to Diff 508076.Mar 24 2023, 6:50 AM

Added FIXME

clang/test/AST/ast-dump-template-decls.cpp
193

Will add a FIXME. Feel free to create a GitHub bug.

aaron.ballman added inline comments.Mar 24 2023, 7:01 AM
clang/test/AST/ast-dump-template-decls.cpp
193

More test coverage is usually a good thing, but in this case, I'd prefer we either fixed the issue or filed a bug (having a test file with a FIXME comment but no issue basically means the issue will be ignored forever in practice, so we're running a test with very little benefit beyond verifying there's not a crash).

tomasz-kaminski-sonarsource edited the summary of this revision. (Show Details)Mar 24 2023, 7:18 AM

This is now tracked as https://github.com/llvm/llvm-project/issues/61680. Let me know if I should refer it in the test.

aaron.ballman accepted this revision.Mar 24 2023, 7:26 AM
In D146784#4219469, @tomasz-kaminski-sonarsource wrote:

This is now tracked as https://github.com/llvm/llvm-project/issues/61680. Let me know if I should refer it in the test.

Thanks, the issue LG! Let's go ahead and mention it here in the test so it's clear this test relates to that issue. LGTM with that handled.

This revision is now accepted and ready to land.Mar 24 2023, 7:26 AM
tomasz-kaminski-sonarsource updated this revision to Diff 508088.Mar 24 2023, 7:27 AM

Updated FIXME with GitHub bug reference.

tomasz-kaminski-sonarsource updated this revision to Diff 508091.Mar 24 2023, 7:35 AM
This comment was removed by tomasz-kaminski-sonarsource.
tomasz-kaminski-sonarsource updated this revision to Diff 508102.Mar 24 2023, 8:05 AM

Reverted accidentally included changes.

tomasz-kaminski-sonarsource updated this revision to Diff 508104.Mar 24 2023, 8:12 AM

Reintroduced bug commit refernces. Sorry for the noise, but I still sometimes get confused with arc.

Harbormaster completed remote builds in B221598: Diff 508104.Mar 25 2023, 9:32 AM
This revision was landed with ongoing or failed builds.Mar 27 2023, 1:11 AM
Closed by commit rG4a9dc94d336a: [clang] Test for AST Dumping of the template variables (authored by tomasz-kaminski-sonarsource). · Explain Why
This revision was automatically updated to reflect the committed changes.
tomasz-kaminski-sonarsource added a commit: rG4a9dc94d336a: [clang] Test for AST Dumping of the template variables.

Revision Contents

PathSize
clang/
test/
AST/
78 lines

Diff 507973

clang/test/AST/ast-dump-template-decls.cpp

// Test without serialization: // Test without serialization:
// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown -ast-dump %s \ // RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown -ast-dump %s \
// RUN: | FileCheck -strict-whitespace %s // RUN: | FileCheck -strict-whitespace %s --check-prefix=DIRECT
// //
// Test with serialization: // Test with serialization:
// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown -emit-pch -o %t %s // RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown -emit-pch -o %t %s
// RUN: %clang_cc1 -x c++ -std=c++17 -triple x86_64-unknown-unknown -include-pch %t -ast-dump-all /dev/null \ // RUN: %clang_cc1 -x c++ -std=c++17 -triple x86_64-unknown-unknown -include-pch %t -ast-dump-all /dev/null \
// RUN: | sed -e "s/ <undeserialized declarations>//" -e "s/ imported//" \ // RUN: | sed -e "s/ <undeserialized declarations>//" -e "s/ imported//" \
// RUN: | FileCheck --strict-whitespace %s // RUN: | FileCheck --strict-whitespace %s --check-prefix=SERIALIZED
template <typename Ty> template <typename Ty>
// CHECK: FunctionTemplateDecl 0x{{[^ ]*}} <{{.*}}:1, line:[[@LINE+2]]:10> col:6 a // CHECK: FunctionTemplateDecl 0x{{[^ ]*}} <{{.*}}:1, line:[[@LINE+2]]:10> col:6 a
// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}} <line:[[@LINE-2]]:11, col:20> col:20 referenced typename depth 0 index 0 Ty // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}} <line:[[@LINE-2]]:11, col:20> col:20 referenced typename depth 0 index 0 Ty
void a(Ty); void a(Ty);
template <typename... Ty> template <typename... Ty>
// CHECK: FunctionTemplateDecl 0x{{[^ ]*}} <line:[[@LINE-1]]:1, line:[[@LINE+2]]:13> col:6 b // CHECK: FunctionTemplateDecl 0x{{[^ ]*}} <line:[[@LINE-1]]:1, line:[[@LINE+2]]:13> col:6 b
▲ Show 20 Lines • Show All 155 Lines • ▼ Show 20 Lines
// CHECK: TemplateSpecializationType 0x{{[^ ]*}} 'A<int>' sugar A // CHECK: TemplateSpecializationType 0x{{[^ ]*}} 'A<int>' sugar A
// CHECK-NEXT: |-TemplateArgument type 'int':'int' // CHECK-NEXT: |-TemplateArgument type 'int':'int'
// CHECK-NEXT: | `-SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar class depth 0 index 1 D2 // CHECK-NEXT: | `-SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar class depth 0 index 1 D2
// CHECK-NEXT: | |-TypeAliasTemplate 0x{{[^ ]*}} 'D' // CHECK-NEXT: | |-TypeAliasTemplate 0x{{[^ ]*}} 'D'
// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'int' // CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'int'
// CHECK-NEXT: `-RecordType 0x{{[^ ]*}} 'subst_default_argument::A<int>' // CHECK-NEXT: `-RecordType 0x{{[^ ]*}} 'subst_default_argument::A<int>'
// CHECK-NEXT: `-ClassTemplateSpecialization 0x{{[^ ]*}} 'A' // CHECK-NEXT: `-ClassTemplateSpecialization 0x{{[^ ]*}} 'A'
} // namespace subst_default_argument } // namespace subst_default_argument
namespace D146733 {
template<class T>
T unTempl = 1;
// CHECK:VarTemplateDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:13> col:3 unTempl
// CHECK-NEXT: |-TemplateTypeParmDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:10, col:16> col:16 referenced class depth 0 index 0 T
// CHECK-NEXT: |-VarDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, col:13> col:3 unTempl 'T' cinit
// CHECK-NEXT: | `-IntegerLiteral 0x{{[^ ]*}} <col:13> 'int' 1
template<>
int unTempl<int>;
// DIRECT: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:16> col:5 unTempl 'int'
// SERIALIZED: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:5> col:5 unTempl 'int'
erichkeaneUnsubmitted
Not Done
ReplyInline Actions

Hmm... thats curious. We shouldn't commit this unless there is a 'fixme' on it showing we don't really mean it, but in reality, this just deserves a Github bug.

erichkeane: Hmm... thats curious. We shouldn't commit this unless there is a 'fixme' on it showing we…
tomasz-kaminski-sonarsourceAuthorUnsubmitted
Done
ReplyInline Actions

Will add a FIXME. Feel free to create a GitHub bug.

tomasz-kaminski-sonarsource: Will add a FIXME. Feel free to create a GitHub bug.
aaron.ballmanUnsubmitted
Not Done
ReplyInline Actions

More test coverage is usually a good thing, but in this case, I'd prefer we either fixed the issue or filed a bug (having a test file with a FIXME comment but no issue basically means the issue will be ignored forever in practice, so we're running a test with very little benefit beyond verifying there's not a crash).

aaron.ballman: More test coverage is usually a good thing, but in this case, I'd prefer we either fixed the…
// CHECK-NEXT: `-TemplateArgument type 'int'
// CHECK-NEXT: `-BuiltinType 0x{{[^ ]*}} 'int'
template<>
float unTempl<float> = 1;
// DIRECT: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:20> col:7 unTempl 'float' cinit
// SERIALIZED: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:24> col:7 unTempl 'float' cinit
// CHECK-NEXT: |-TemplateArgument type 'float'
// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'float'
// CHECK-NEXT: `-ImplicitCastExpr 0x{{[^ ]*}} <col:24> 'float' <IntegralToFloating>
// CHECK-NEXT: `-IntegerLiteral 0x{{[^ ]*}} <col:24> 'int' 1
template<class T, class U>
T binTempl = 1;
// CHECK: VarTemplateDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:14> col:3 binTempl
// CHECK-NEXT: |-TemplateTypeParmDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:10, col:16> col:16 referenced class depth 0 index 0 T
// CHECK-NEXT: |-TemplateTypeParmDecl 0x{{[^ ]*}} <col:19, col:25> col:25 class depth 0 index 1 U
// CHECK-NEXT: |-VarDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, col:14> col:3 binTempl 'T' cinit
// CHECK-NEXT: | `-IntegerLiteral 0x{{[^ ]*}} <col:14> 'int' 1
template<class U>
int binTempl<int, U>;
// DIRECT: VarTemplatePartialSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:20> col:5 binTempl 'int'
// SERIALIZED: VarTemplatePartialSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:20> col:5 binTempl 'int'
// CHECK-NEXT: |-TemplateTypeParmDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:10, col:16> col:16 referenced class depth 0 index 0 U
// CHECK-NEXT: |-TemplateArgument type 'int'
// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'int'
// CHECK-NEXT: `-TemplateArgument type 'type-parameter-0-0'
// CHECK-NEXT: `-TemplateTypeParmType 0x{{[^ ]*}} 'type-parameter-0-0' dependent depth 0 index 0
template<class U>
float binTempl<float, U> = 1;
// CHECK: VarTemplatePartialSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:24> col:7 binTempl 'float' cinit
// CHECK-NEXT: |-TemplateTypeParmDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:10, col:16> col:16 referenced class depth 0 index 0 U
// CHECK-NEXT: |-TemplateArgument type 'float'
// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'float'
// CHECK-NEXT: |-TemplateArgument type 'type-parameter-0-0'
// CHECK-NEXT: | `-TemplateTypeParmType 0x{{[^ ]*}} 'type-parameter-0-0' dependent depth 0 index 0
// CHECK-NEXT: `-ImplicitCastExpr 0x{{[^ ]*}} <line:{{[0-9]+}}:28> 'float' <IntegralToFloating>
// CHECK-NEXT: `-IntegerLiteral 0x{{[^ ]*}} <col:28> 'int' 1
template<>
int binTempl<int, int>;
// DIRECT: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:22> col:5 binTempl 'int'
// SERIALIZED: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:5> col:5 binTempl 'int'
// CHECK-NEXT: |-TemplateArgument type 'int'
// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'int'
// CHECK-NEXT: `-TemplateArgument type 'int'
// CHECK-NEXT: `-BuiltinType 0x{{[^ ]*}} 'int'
template<>
float binTempl<float, float> = 1;
// DIRECT: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:28> col:7 binTempl 'float' cinit
// SERIALIZED: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:32> col:7 binTempl 'float' cinit
// CHECK-NEXT: |-TemplateArgument type 'float'
// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'float'
// CHECK-NEXT: |-TemplateArgument type 'float'
// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'float'
// CHECK-NEXT: `-ImplicitCastExpr 0x{{[^ ]*}} <col:32> 'float' <IntegralToFloating>
// CHECK-NEXT: `-IntegerLiteral 0x{{[^ ]*}} <col:32> 'int' 1
}