[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
https://github.com/jcsxky closed https://github.com/llvm/llvm-project/pull/74919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
https://github.com/balazske approved this pull request. https://github.com/llvm/llvm-project/pull/74919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
https://github.com/jcsxky updated
https://github.com/llvm/llvm-project/pull/74919
>From 583cbd47533ff1aa71874c502affc44ce5b5c107 Mon Sep 17 00:00:00 2001
From: huqizhi
Date: Sat, 9 Dec 2023 12:00:02 +0800
Subject: [PATCH] [clang][ASTImporter] skip TemplateTypeParmDecl in
VisitTypeAliasTemplateDecl
---
clang/lib/AST/ASTImporter.cpp | 9 +++--
clang/lib/AST/ASTStructuralEquivalence.cpp | 12 ++
clang/unittests/AST/ASTImporterTest.cpp| 47 ++
3 files changed, 64 insertions(+), 4 deletions(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a4..270574a7704505 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2771,9 +2771,11 @@
ASTNodeImporter::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
- if (auto *FoundAlias = dyn_cast(FoundDecl))
-return Importer.MapImported(D, FoundAlias);
- ConflictingDecls.push_back(FoundDecl);
+ if (auto *FoundAlias = dyn_cast(FoundDecl)) {
+if (IsStructuralMatch(D, FoundAlias))
+ return Importer.MapImported(D, FoundAlias);
+ConflictingDecls.push_back(FoundDecl);
+ }
}
if (!ConflictingDecls.empty()) {
@@ -9391,7 +9393,6 @@ Expected ASTImporter::Import(Decl *FromD) {
setImportDeclError(FromD, *Error);
return make_error(*Error);
}
-
// Make sure that ImportImpl registered the imported decl.
assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
if (auto Error = ImportAttrs(ToD, FromD))
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 6bb4bf14b873d7..1f492b051e0341 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1977,6 +1977,18 @@ static bool
IsStructurallyEquivalent(StructuralEquivalenceContext ,
D2->getTemplatedDecl()->getType());
}
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
+ TypeAliasTemplateDecl *D1,
+ TypeAliasTemplateDecl *D2) {
+ // Check template parameters.
+ if (!IsTemplateDeclCommonStructurallyEquivalent(Context, D1, D2))
+return false;
+
+ // Check the templated declaration.
+ return IsStructurallyEquivalent(Context, D1->getTemplatedDecl(),
+ D2->getTemplatedDecl());
+}
+
static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
ConceptDecl *D1,
ConceptDecl *D2) {
diff --git a/clang/unittests/AST/ASTImporterTest.cpp
b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf8..2328e98a663810 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9284,6 +9284,53 @@ TEST_P(ASTImporterOptionSpecificTestBase,
// EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
}
+TEST_P(ASTImporterOptionSpecificTestBase,
+ ImportTypeAliasTemplateAfterSimilarCalledTemplateTypeParm) {
+ const char *Code =
+ R"(
+ struct S;
+ template
+ using Callable = S;
+ template
+ int bindingFunctionVTable;
+ )";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX17);
+
+ auto *FromCallable = FirstDeclMatcher().match(
+ FromTU, typeAliasTemplateDecl(hasName("Callable")));
+
+ auto *FromCallableParm = FirstDeclMatcher().match(
+ FromTU, templateTypeParmDecl(hasName("Callable")));
+
+ auto *ToFromCallableParm = Import(FromCallableParm, Lang_CXX17);
+ auto *ToCallable = Import(FromCallable, Lang_CXX17);
+ EXPECT_TRUE(ToFromCallableParm);
+ EXPECT_TRUE(ToCallable);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportConflictTypeAliasTemplate) {
+ const char *ToCode =
+ R"(
+ struct S;
+ template
+ using Callable = S;
+ )";
+ const char *Code =
+ R"(
+ struct S;
+ template
+ using Callable = S;
+ )";
+ (void)getToTuDecl(ToCode, Lang_CXX17);
+ Decl *FromTU = getTuDecl(Code, Lang_CXX17);
+
+ auto *FromCallable = FirstDeclMatcher().match(
+ FromTU, typeAliasTemplateDecl(hasName("Callable")));
+
+ auto *ImportedCallable = Import(FromCallable, Lang_CXX17);
+ EXPECT_FALSE(ImportedCallable);
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
@@ -1977,6 +1977,22 @@ static bool
IsStructurallyEquivalent(StructuralEquivalenceContext ,
D2->getTemplatedDecl()->getType());
}
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
+ TypeAliasTemplateDecl *D1,
+ TypeAliasTemplateDecl *D2) {
balazske wrote:
This function should work if similar to the one with `ClassTemplateDecl` (use
`IsTemplateDeclCommonStructurallyEquivalent`).
https://github.com/llvm/llvm-project/pull/74919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
balazske wrote: > > Is import of `Callable` should be failed? I compiled this code > > ```c++ > struct S; > template > using Callable = S; > template > using Callable = S; > ``` > > and clang report an error. Yes the test was not exact, with the new code this import should fail. https://github.com/llvm/llvm-project/pull/74919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
jcsxky wrote:
> The `VisitTypeAliasTemplateDecl` function should be re-designed to check for
> structural equivalence at import. The following test does not pass because an
> existing `TypeAliasTemplateDecl` declaration with the same name is always
> found and returned, without check for structural equivalence.
>
> ```
> TEST_P(ASTImporterOptionSpecificTestBase, ImportTypeAliasTemplateDecl1) {
> const char *ToCode =
> R"(
> struct S;
> template
> using Callable = S;
> )";
> const char *Code =
> R"(
> struct S;
> template
> using Callable = S;
> )";
> Decl *ToTU = getToTuDecl(ToCode, Lang_CXX17);
> Decl *FromTU = getTuDecl(Code, Lang_CXX17);
>
> auto *FromCallable = FirstDeclMatcher().match(
> FromTU, typeAliasTemplateDecl(hasName("Callable")));
>
> auto *ToCallable = FirstDeclMatcher().match(
> ToTU, typeAliasTemplateDecl(hasName("Callable")));
>
> auto *ImportedCallable = Import(FromCallable, Lang_CXX17);
> EXPECT_TRUE(ImportedCallable);
> EXPECT_NE(ImportedCallable, ToCallable);
> }
> ```
>
> Additionally I discovered that import of `ClassTemplateDecl` is not correct
> too: If there is an object with the same name that is not a
> `ClassTemplateDecl`, it is just ignored at import. This is not correct, the
> existing object may cause name conflict (for example it can be a non-template
> `RecordDecl`). (I found this when checking the questions in my last comment.)
> This is an independent problem but should be fixed.
Is import of `Callable` should be failed? I compiled this code
```cpp
struct S;
template
using Callable = S;
template
using Callable = S;
```
and clang report an error.
https://github.com/llvm/llvm-project/pull/74919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
https://github.com/jcsxky updated
https://github.com/llvm/llvm-project/pull/74919
>From a2dcc7f471237e78f374c204216c6574059aa950 Mon Sep 17 00:00:00 2001
From: huqizhi
Date: Sat, 9 Dec 2023 12:00:02 +0800
Subject: [PATCH] [clang][ASTImporter] skip TemplateTypeParmDecl in
VisitTypeAliasTemplateDecl
---
clang/lib/AST/ASTImporter.cpp | 9 +++--
clang/lib/AST/ASTStructuralEquivalence.cpp | 16
clang/unittests/AST/ASTImporterTest.cpp| 47 ++
3 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a4..270574a7704505 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2771,9 +2771,11 @@
ASTNodeImporter::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
- if (auto *FoundAlias = dyn_cast(FoundDecl))
-return Importer.MapImported(D, FoundAlias);
- ConflictingDecls.push_back(FoundDecl);
+ if (auto *FoundAlias = dyn_cast(FoundDecl)) {
+if (IsStructuralMatch(D, FoundAlias))
+ return Importer.MapImported(D, FoundAlias);
+ConflictingDecls.push_back(FoundDecl);
+ }
}
if (!ConflictingDecls.empty()) {
@@ -9391,7 +9393,6 @@ Expected ASTImporter::Import(Decl *FromD) {
setImportDeclError(FromD, *Error);
return make_error(*Error);
}
-
// Make sure that ImportImpl registered the imported decl.
assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
if (auto Error = ImportAttrs(ToD, FromD))
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 6bb4bf14b873d7..ae55ff7811e794 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1977,6 +1977,22 @@ static bool
IsStructurallyEquivalent(StructuralEquivalenceContext ,
D2->getTemplatedDecl()->getType());
}
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
+ TypeAliasTemplateDecl *D1,
+ TypeAliasTemplateDecl *D2) {
+ if (!IsStructurallyEquivalent(Context, D1->getDeclName(), D2->getDeclName()))
+return false;
+
+ // Check the templated declaration.
+ if (!IsStructurallyEquivalent(Context, D1->getTemplatedDecl(),
+D2->getTemplatedDecl()))
+return false;
+
+ // Check template parameters.
+ return IsStructurallyEquivalent(Context, D1->getTemplateParameters(),
+ D2->getTemplateParameters());
+}
+
static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
ConceptDecl *D1,
ConceptDecl *D2) {
diff --git a/clang/unittests/AST/ASTImporterTest.cpp
b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf8..2328e98a663810 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9284,6 +9284,53 @@ TEST_P(ASTImporterOptionSpecificTestBase,
// EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
}
+TEST_P(ASTImporterOptionSpecificTestBase,
+ ImportTypeAliasTemplateAfterSimilarCalledTemplateTypeParm) {
+ const char *Code =
+ R"(
+ struct S;
+ template
+ using Callable = S;
+ template
+ int bindingFunctionVTable;
+ )";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX17);
+
+ auto *FromCallable = FirstDeclMatcher().match(
+ FromTU, typeAliasTemplateDecl(hasName("Callable")));
+
+ auto *FromCallableParm = FirstDeclMatcher().match(
+ FromTU, templateTypeParmDecl(hasName("Callable")));
+
+ auto *ToFromCallableParm = Import(FromCallableParm, Lang_CXX17);
+ auto *ToCallable = Import(FromCallable, Lang_CXX17);
+ EXPECT_TRUE(ToFromCallableParm);
+ EXPECT_TRUE(ToCallable);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportConflictTypeAliasTemplate) {
+ const char *ToCode =
+ R"(
+ struct S;
+ template
+ using Callable = S;
+ )";
+ const char *Code =
+ R"(
+ struct S;
+ template
+ using Callable = S;
+ )";
+ (void)getToTuDecl(ToCode, Lang_CXX17);
+ Decl *FromTU = getTuDecl(Code, Lang_CXX17);
+
+ auto *FromCallable = FirstDeclMatcher().match(
+ FromTU, typeAliasTemplateDecl(hasName("Callable")));
+
+ auto *ImportedCallable = Import(FromCallable, Lang_CXX17);
+ EXPECT_FALSE(ImportedCallable);
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
https://github.com/jcsxky updated
https://github.com/llvm/llvm-project/pull/74919
>From e4e981ed4f545f3dd4cc709bab30468a8ceb3962 Mon Sep 17 00:00:00 2001
From: huqizhi
Date: Sat, 9 Dec 2023 12:00:02 +0800
Subject: [PATCH] [clang][ASTImporter] skip TemplateTypeParmDecl in
VisitTypeAliasTemplateDecl
---
clang/lib/AST/ASTImporter.cpp | 16 ---
clang/lib/AST/ASTStructuralEquivalence.cpp | 16 +++
clang/unittests/AST/ASTImporterTest.cpp| 50 ++
3 files changed, 77 insertions(+), 5 deletions(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a4..40e5ac6ecb13e4 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2771,9 +2771,12 @@
ASTNodeImporter::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
- if (auto *FoundAlias = dyn_cast(FoundDecl))
-return Importer.MapImported(D, FoundAlias);
- ConflictingDecls.push_back(FoundDecl);
+ if (auto *FoundAlias = dyn_cast(FoundDecl)) {
+// if (IsStructuralMatch(D,FoundAlias))
+ return Importer.MapImported(D, FoundAlias);
+
+ConflictingDecls.push_back(FoundDecl);
+ }
}
if (!ConflictingDecls.empty()) {
@@ -9073,7 +9076,6 @@ class AttrImporter {
ToAttr = FromAttr->clone(Importer.getToContext());
ToAttr->setRange(ToRange);
-ToAttr->setAttrName(Importer.Import(FromAttr->getAttrName()));
}
// Get the result of the previous import attempt (can be used only once).
@@ -9223,6 +9225,11 @@ Expected ASTImporter::Import(const Attr
*FromAttr) {
AI.castAttrAs()->setCountedByFieldLoc(SR.get());
break;
}
+ case attr::AlignValue:{
+auto *From = cast(FromAttr);
+AI.importAttr(From,AI.importArg(From->getAlignment()).value());
+break;
+ }
default: {
// The default branch works for attributes that have no arguments to
import.
@@ -9391,7 +9398,6 @@ Expected ASTImporter::Import(Decl *FromD) {
setImportDeclError(FromD, *Error);
return make_error(*Error);
}
-
// Make sure that ImportImpl registered the imported decl.
assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
if (auto Error = ImportAttrs(ToD, FromD))
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 6bb4bf14b873d7..5d6c3f35e50832 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1977,6 +1977,22 @@ static bool
IsStructurallyEquivalent(StructuralEquivalenceContext ,
D2->getTemplatedDecl()->getType());
}
+// static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
+// TypeAliasTemplateDecl *D1,
+// TypeAliasTemplateDecl *D2) {
+// if (!IsStructurallyEquivalent(Context, D1->getDeclName(),
D2->getDeclName()))
+// return false;
+
+// // Check the templated declaration.
+// if (!IsStructurallyEquivalent(Context, D1->getTemplatedDecl(),
+// D2->getTemplatedDecl()))
+// return false;
+
+// // Check template parameters.
+// return IsStructurallyEquivalent(Context, D1->getTemplateParameters(),
+// D2->getTemplateParameters());
+// }
+
static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
ConceptDecl *D1,
ConceptDecl *D2) {
diff --git a/clang/unittests/AST/ASTImporterTest.cpp
b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf8..454a4d42d77520 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9284,6 +9284,56 @@ TEST_P(ASTImporterOptionSpecificTestBase,
// EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
}
+// TEST_P(ASTImporterOptionSpecificTestBase, ImportTypeAliasTemplateDecl) {
+// const char *Code =
+// R"(
+// struct S;
+// template
+// using Callable = S;
+// template
+// int bindingFunctionVTable;
+// )";
+// Decl *FromTU = getTuDecl(Code, Lang_CXX17);
+
+// auto *FromCallable1 = FirstDeclMatcher().match(
+// FromTU, typeAliasTemplateDecl(hasName("Callable")));
+
+// auto *FromCallable2 = FirstDeclMatcher().match(
+// FromTU, templateTypeParmDecl(hasName("Callable")));
+
+// auto *ToCallable2 = Import(FromCallable2, Lang_CXX17);
+// auto *ToCallable1 = Import(FromCallable1, Lang_CXX17);
+// EXPECT_TRUE(ToCallable1);
+// EXPECT_TRUE(ToCallable2);
+// }
+
+// TEST_P(ASTImporterOptionSpecificTestBase, X) {
+// const char *CodeFrom1 =
+// R"(
+// template
+// bool A = A;
+// )";
+// const char *CodeFrom2 =
+// R"(
+// template
+//
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
@@ -9284,6 +9284,29 @@ TEST_P(ASTImporterOptionSpecificTestBase,
// EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
}
+TEST_P(ASTImporterOptionSpecificTestBase, ImportTypeAliasTemplateDecl) {
balazske wrote:
I like better names for the test
(`ImportTypeAliasTemplateAfterSimilarCalledTemplateTypeParm`) and `Callable1`
(`Callable`), `Callable2` (`CallableParm`).
https://github.com/llvm/llvm-project/pull/74919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
balazske wrote:
This code for the loop in `VisitTypeAliasTemplateDecl` should work:
```
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
if (auto *FoundAlias = dyn_cast(FoundDecl)) {
if (IsStructuralMatch(D, FoundAlias))
return Importer.MapImported(D, FoundAlias);
ConflictingDecls.push_back(FoundDecl);
}
}
```
https://github.com/llvm/llvm-project/pull/74919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
balazske wrote: `VisitTypeAliasTemplateDecl` looks wrong and some fix should be made, this PR is good for the fix. This import function is different from the others because `ConflictingDecls` is populated when an object with different type is found, but this is the correct way. I think that `TemplateTypeParmDecl` objects are found with the lookup. A good fix would be to change the loop in `VisitTypeAliasTemplateDecl` to check for structural equivalence too, if equivalent is found use `MapImported`, otherwise add to `ConflictingDecls`. But skip all found objects that are not `TypeAliasTemplateDecl`. This makes it similar to the others, even if not correct, the remaining problem can be fixed later (and looks less important). https://github.com/llvm/llvm-project/pull/74919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
jcsxky wrote:
> The problem may be related to the fact that template parameter declarations
> can have the `TranslationUnitDecl` as parent until the template (with these
> parameters) is finally created. In the temporary phase the object
> (`TemplateTypeParmDecl`) is found with lookup if it has the same name as an
> other imported object. Does the crash happen if in the test code the
> `TypeAliasTemplateDecl` is replaced with a plain `ClassTemplateDecl` or
> `FunctionTemplateDecl`? If yes the change is needed at these import functions
> too.
Replace `TypeAliasTemplateDecl` with `ClassTemplateDecl` or
`FunctionTemplateDecl` has no problem and passed the test with following test
code
```cpp
struct S;
template
class Callable {};
template
int bindingFunctionVTable;
```
and
```cpp
struct S;
template
void Callable(){}
template
int bindingFunctionVTable;
```
Test code seems good to current issue and the two you mentioned above can't be
fixed with this pr. Should I abandon this pr or fix current issue only first?
https://github.com/llvm/llvm-project/pull/74919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
jcsxky wrote:
> The `VisitTypeAliasTemplateDecl` function should be re-designed to check for
> structural equivalence at import. The following test does not pass because an
> existing `TypeAliasTemplateDecl` declaration with the same name is always
> found and returned, without check for structural equivalence.
>
> ```
> TEST_P(ASTImporterOptionSpecificTestBase, ImportTypeAliasTemplateDecl1) {
> const char *ToCode =
> R"(
> struct S;
> template
> using Callable = S;
> )";
> const char *Code =
> R"(
> struct S;
> template
> using Callable = S;
> )";
> Decl *ToTU = getToTuDecl(ToCode, Lang_CXX17);
> Decl *FromTU = getTuDecl(Code, Lang_CXX17);
>
> auto *FromCallable = FirstDeclMatcher().match(
> FromTU, typeAliasTemplateDecl(hasName("Callable")));
>
> auto *ToCallable = FirstDeclMatcher().match(
> ToTU, typeAliasTemplateDecl(hasName("Callable")));
>
> auto *ImportedCallable = Import(FromCallable, Lang_CXX17);
> EXPECT_TRUE(ImportedCallable);
> EXPECT_NE(ImportedCallable, ToCallable);
> }
> ```
>
> Additionally I discovered that import of `ClassTemplateDecl` is not correct
> too: If there is an object with the same name that is not a
> `ClassTemplateDecl`, it is just ignored at import. This is not correct, the
> existing object may cause name conflict (for example it can be a non-template
> `RecordDecl`). (I found this when checking the questions in my last comment.)
> This is an independent problem but should be fixed.
I pulled the latest code of main branch and found it seems incorrect with the
test case I have supplied. I will have a double check this pr and issues you
mentioned here. Thank for your detail comments and guidance.
https://github.com/llvm/llvm-project/pull/74919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
balazske wrote:
The `VisitTypeAliasTemplateDecl` function should be re-designed to check for
structural equivalence at import. The following test does not pass because an
existing `TypeAliasTemplateDecl` declaration with the same name is always found
and returned, without check for structural equivalence.
```
TEST_P(ASTImporterOptionSpecificTestBase, ImportTypeAliasTemplateDecl1) {
const char *ToCode =
R"(
struct S;
template
using Callable = S;
)";
const char *Code =
R"(
struct S;
template
using Callable = S;
)";
Decl *ToTU = getToTuDecl(ToCode, Lang_CXX17);
Decl *FromTU = getTuDecl(Code, Lang_CXX17);
auto *FromCallable = FirstDeclMatcher().match(
FromTU, typeAliasTemplateDecl(hasName("Callable")));
auto *ToCallable = FirstDeclMatcher().match(
ToTU, typeAliasTemplateDecl(hasName("Callable")));
auto *ImportedCallable = Import(FromCallable, Lang_CXX17);
EXPECT_TRUE(ImportedCallable);
EXPECT_NE(ImportedCallable, ToCallable);
}
```
Additionally I discovered that import of `ClassTemplateDecl` is not correct
too: If there is an object with the same name that is not a
`ClassTemplateDecl`, it is just ignored at import. This is not correct, the
existing object may cause name conflict (for example it can be a non-template
`RecordDecl`). (I found this when checking the questions in my last comment.)
This is an independent problem but should be fixed.
https://github.com/llvm/llvm-project/pull/74919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
balazske wrote: The problem may be related to the fact that template parameter declarations can have the `TranslationUnitDecl` as parent until the template (with these parameters) is finally created. In the temporary phase the object (`TemplateTypeParmDecl`) is found with lookup if it has the same name as an other imported object. Does the crash happen if in the test code the `TypeAliasTemplateDecl` is replaced with a plain `ClassTemplateDecl` or `FunctionTemplateDecl`? If yes the change is needed at these import functions too. https://github.com/llvm/llvm-project/pull/74919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
https://github.com/jcsxky updated
https://github.com/llvm/llvm-project/pull/74919
>From e656aa2a3850f845987bb0ddc56b308d22d2dafd Mon Sep 17 00:00:00 2001
From: huqizhi
Date: Sat, 9 Dec 2023 12:00:02 +0800
Subject: [PATCH] [clang][ASTImporter] skip TemplateTypeParmDecl in
VisitTypeAliasTemplateDecl
---
clang/lib/AST/ASTImporter.cpp | 3 ++-
clang/unittests/AST/ASTImporterTest.cpp | 23 +++
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a..bfe9af648e603 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2769,7 +2769,8 @@
ASTNodeImporter::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
unsigned IDNS = Decl::IDNS_Ordinary;
auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
- if (!FoundDecl->isInIdentifierNamespace(IDNS))
+ if (!FoundDecl->isInIdentifierNamespace(IDNS) ||
+ isa_and_nonnull(FoundDecl))
continue;
if (auto *FoundAlias = dyn_cast(FoundDecl))
return Importer.MapImported(D, FoundAlias);
diff --git a/clang/unittests/AST/ASTImporterTest.cpp
b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf..b53cf11f315c8 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9284,6 +9284,29 @@ TEST_P(ASTImporterOptionSpecificTestBase,
// EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
}
+TEST_P(ASTImporterOptionSpecificTestBase, ImportTypeAliasTemplateDecl) {
+ const char *Code =
+ R"(
+ struct S;
+ template
+ using Callable = S;
+ template
+ int bindingFunctionVTable;
+ )";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX17);
+
+ auto *FromCallable1 = FirstDeclMatcher().match(
+ FromTU, typeAliasTemplateDecl(hasName("Callable")));
+
+ auto *FromCallable2 = FirstDeclMatcher().match(
+ FromTU, templateTypeParmDecl(hasName("Callable")));
+
+ auto *ToCallable2 = Import(FromCallable2, Lang_CXX17);
+ auto *ToCallable1 = Import(FromCallable1, Lang_CXX17);
+ EXPECT_TRUE(ToCallable1);
+ EXPECT_TRUE(ToCallable2);
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Qizhi Hu (jcsxky)
Changes
Skip checking `TemplateTypeParmDecl ` in `VisitTypeAliasTemplateDecl`. [Fix
this crash](https://github.com/llvm/llvm-project/issues/74765)
---
Full diff: https://github.com/llvm/llvm-project/pull/74919.diff
2 Files Affected:
- (modified) clang/lib/AST/ASTImporter.cpp (+2-1)
- (modified) clang/unittests/AST/ASTImporterTest.cpp (+23)
``diff
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a4..bfe9af648e603b 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2769,7 +2769,8 @@
ASTNodeImporter::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
unsigned IDNS = Decl::IDNS_Ordinary;
auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
- if (!FoundDecl->isInIdentifierNamespace(IDNS))
+ if (!FoundDecl->isInIdentifierNamespace(IDNS) ||
+ isa_and_nonnull(FoundDecl))
continue;
if (auto *FoundAlias = dyn_cast(FoundDecl))
return Importer.MapImported(D, FoundAlias);
diff --git a/clang/unittests/AST/ASTImporterTest.cpp
b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf8..6f9aba2c867eea 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9284,6 +9284,29 @@ TEST_P(ASTImporterOptionSpecificTestBase,
// EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
}
+TEST_P(ASTImporterOptionSpecificTestBase, XXX) {
+ const char *Code =
+ R"(
+ struct S;
+ template
+ using Callable = S;
+ template
+ int bindingFunctionVTable;
+ )";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX17);
+
+ auto *FromCallable1 = FirstDeclMatcher().match(
+ FromTU, typeAliasTemplateDecl(hasName("Callable")));
+
+ auto *FromCallable2 = FirstDeclMatcher().match(
+ FromTU, templateTypeParmDecl(hasName("Callable")));
+
+ auto *ToCallable2 = Import(FromCallable2, Lang_CXX17);
+ auto *ToCallable1 = Import(FromCallable1, Lang_CXX17);
+ EXPECT_TRUE(ToCallable1);
+ EXPECT_TRUE(ToCallable2);
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
``
https://github.com/llvm/llvm-project/pull/74919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)
https://github.com/jcsxky created
https://github.com/llvm/llvm-project/pull/74919
Skip checking `TemplateTypeParmDecl ` in `VisitTypeAliasTemplateDecl`. [Fix
this crash](https://github.com/llvm/llvm-project/issues/74765)
>From b3c28d66efb98dff8b8f879bda92341bf62f45d3 Mon Sep 17 00:00:00 2001
From: huqizhi
Date: Sat, 9 Dec 2023 12:00:02 +0800
Subject: [PATCH] [clang][ASTImporter] skip TemplateTypeParmDecl in
VisitTypeAliasTemplateDecl
---
clang/lib/AST/ASTImporter.cpp | 3 ++-
clang/unittests/AST/ASTImporterTest.cpp | 23 +++
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a4..bfe9af648e603b 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2769,7 +2769,8 @@
ASTNodeImporter::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
unsigned IDNS = Decl::IDNS_Ordinary;
auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
- if (!FoundDecl->isInIdentifierNamespace(IDNS))
+ if (!FoundDecl->isInIdentifierNamespace(IDNS) ||
+ isa_and_nonnull(FoundDecl))
continue;
if (auto *FoundAlias = dyn_cast(FoundDecl))
return Importer.MapImported(D, FoundAlias);
diff --git a/clang/unittests/AST/ASTImporterTest.cpp
b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf8..6f9aba2c867eea 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9284,6 +9284,29 @@ TEST_P(ASTImporterOptionSpecificTestBase,
// EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
}
+TEST_P(ASTImporterOptionSpecificTestBase, XXX) {
+ const char *Code =
+ R"(
+ struct S;
+ template
+ using Callable = S;
+ template
+ int bindingFunctionVTable;
+ )";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX17);
+
+ auto *FromCallable1 = FirstDeclMatcher().match(
+ FromTU, typeAliasTemplateDecl(hasName("Callable")));
+
+ auto *FromCallable2 = FirstDeclMatcher().match(
+ FromTU, templateTypeParmDecl(hasName("Callable")));
+
+ auto *ToCallable2 = Import(FromCallable2, Lang_CXX17);
+ auto *ToCallable1 = Import(FromCallable1, Lang_CXX17);
+ EXPECT_TRUE(ToCallable1);
+ EXPECT_TRUE(ToCallable2);
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
