[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG07ab5140080e: [clang][ASTImporter]Skip check depth of friend template parameter (authored by jcsxky, committed by wangpc). Repository: rG LLVM Git

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:140 +D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls01, +StructuralEquivalenceKind::Default, false, false, false, +IgnoreTemplateParmDepth); --

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky updated this revision to Diff 552179. jcsxky added a comment. add argument comment according to bugprone-argument-comment . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

shafik added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:140 +D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls01, +StructuralEquivalenceKind::Default, false, false, false, +IgnoreTemplateParmDepth); --

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky added a comment. In D156693#4603537 , @balazske wrote: > My concern was related to nested namespaces or nested classes with friend > declarations that are equivalent and differ only in the nesting level. It may > be possible to construct code whe

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

balazske accepted this revision. balazske added a comment. This revision is now accepted and ready to land. My concern was related to nested namespaces or nested classes with friend declarations that are equivalent and differ only in the nesting level. It may be possible to construct code where

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky added a comment. In D156693#4584657 , @balazske wrote: > A simple test should be added to StructuralEquivalenceTest.cpp too to check > if ignore (and not ignore) depth works. > > I think this solution is not always correct, but is still an improve

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky updated this revision to Diff 550352. jcsxky added a comment. Fix variable names according to the convention. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156693/new/ https://reviews.llvm.org/D156693 Files: clang/include/clang/AST/ASTStr

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

balazske added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1716 + IgnoreTemplateParmDepthAtNonTypeTemplateParmDecl) { + auto t = makeDecls( + R"( The convention is to start variable names with uppercase, for example

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky updated this revision to Diff 550214. jcsxky added a comment. Add testcase to illustrate the difference between `TemplateTypeParm` and `NonTypeTemplateParm` in detecting by structural equivalence. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1709 + classTemplateDecl(hasName("A"))); + EXPECT_TRUE(testStructuralMatch(t)); + EXPECT_TRUE(testStructuralMatch(t, true)); balazske wrote: > The intent was to hav

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

balazske added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1709 + classTemplateDecl(hasName("A"))); + EXPECT_TRUE(testStructuralMatch(t)); + EXPECT_TRUE(testStructuralMatch(t, true)); The intent was to have `EXPECT_FALSE`

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky updated this revision to Diff 550208. jcsxky added a comment. Add more simple testcase to StructuralEquivalenceTest.cpp. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156693/new/ https://reviews.llvm.org/D156693 Files: clang/include/clang

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

balazske added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1697 +TEST_F(StructuralEquivalenceTemplateTest, + ClassTemplateEquivalentFriendClassTemplate) { + auto t = makeDecls( IgnoreTemplateParmDepth) {

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:5831 +FoundTemplate->getFriendObjectKind() != Decl::FOK_None && +!D->specializations().empty(); +if (IsStructuralMatch(D, FoundTemplate, true, IgnoreDepth)) {

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

jcsxky updated this revision to Diff 550030. jcsxky added a comment. fixed according to the review and add testcase to StructuralEquivalenceTest.cpp. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156693/new/ https://reviews.llvm.org/D156693 Files:

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

balazske added a comment. A simple test should be added to StructuralEquivalenceTest.cpp too to check if ignore (and not ignore) depth works. I think this solution is not always correct, but is still an improvement. Comment at: clang/include/clang/AST/ASTStructuralEquivalence