[clang] [Serialization] Handle uninitialized type constraints (PR #110496)
falbrechtskirchinger wrote: > @falbrechtskirchinger Ooops, yes, it's true that the bug is gone w/ #79. > > But I'd say if we're going to cherry-pick #79, we have to backport its > precedent #103867 as well, so I think we can probably go ahead with this one > and backport it to 19, and then we can revert this one on the trunk - this is > indeed a bit awkward, but it depends whether it's worth backporting #103867 > @cor3ntin I'd like this crash to be fixed in 19.x. > (Again, I apologize for not looking into it so carefully) No worries. To summarize: Land this PR and backport it. Revert on trunk. Submit new PR for the regression test? (We want to keep that, right?) https://github.com/llvm/llvm-project/pull/110496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Handle uninitialized type constraints (PR #110496)
falbrechtskirchinger wrote: @zyn0217 #79 has fixed this crash as well. Should I strip this PR down to the unit test, or does it still make sense to you, to keep the changes? > Will this be backported to 19.x afterwards? I was also going to ask, though, it's now a question about #79. https://github.com/llvm/llvm-project/pull/110496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Handle uninitialized type constraints (PR #110496)
falbrechtskirchinger wrote: > So yes, this one is still necessary - it should occur with an > assertion-enabled build when compiling a header containing an invalid concept > decl. Not in my testing. https://github.com/llvm/llvm-project/pull/110496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Handle uninitialized type constraints (PR #110496)
@@ -1899,7 +1899,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
Record.push_back(D->wasDeclaredWithTypename());
const TypeConstraint *TC = D->getTypeConstraint();
- assert((bool)TC == D->hasTypeConstraint());
+ Record.push_back(/*TypeConstraintInitialized=*/TC != nullptr);
if (TC) {
falbrechtskirchinger wrote:
Because `D->hasTypeConstraint()` does not imply that `D->getTypeConstraint()`
is non-null, as the code assumed (hence, the assertion).
https://github.com/llvm/llvm-project/pull/110496
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Handle uninitialized type constraints (PR #110496)
@@ -2674,7 +2674,7 @@ void
ASTDeclReader::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
D->setDeclaredWithTypename(Record.readInt());
- if (D->hasTypeConstraint()) {
+ if (Record.readBool() && D->hasTypeConstraint()) {
falbrechtskirchinger wrote:
Sure. Just waiting on a final verdict, on whether this is still needed.
https://github.com/llvm/llvm-project/pull/110496
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Handle uninitialized type constraints (PR #110496)
https://github.com/falbrechtskirchinger updated
https://github.com/llvm/llvm-project/pull/110496
>From 220d76a2b2327318a6b5261896736ddd3f05988a Mon Sep 17 00:00:00 2001
From: Florian Albrechtskirchinger
Date: Mon, 30 Sep 2024 12:58:15 +0200
Subject: [PATCH] [Serialization] Handle uninitialized type constraints
The ASTWriter currently assumes template type constraints to be
initialized ((bool)getTypeConstraint() == hasTypeConstraint()).
The attached test case presents a scenario where that is not the case.
This patch removes the assumption and adds another boolean to the
serialization, to explicitly encode whether the type constraint has been
initialized.
Fixes #99036
Fixes #109354
---
clang/lib/Serialization/ASTReaderDecl.cpp | 2 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 5 +++--
clang/test/PCH/cxx2a-constraints-crash.cpp | 18 ++
3 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 7cead2728ca938..90783963934bb0 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2674,7 +2674,7 @@ void
ASTDeclReader::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
D->setDeclaredWithTypename(Record.readInt());
- if (D->hasTypeConstraint()) {
+ if (Record.readBool() && D->hasTypeConstraint()) {
ConceptReference *CR = nullptr;
if (Record.readBool())
CR = Record.readConceptReference();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index b71684569609ac..4da2108f553127 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1899,7 +1899,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
Record.push_back(D->wasDeclaredWithTypename());
const TypeConstraint *TC = D->getTypeConstraint();
- assert((bool)TC == D->hasTypeConstraint());
+ Record.push_back(/*TypeConstraintInitialized=*/TC != nullptr);
if (TC) {
auto *CR = TC->getConceptReference();
Record.push_back(CR != nullptr);
@@ -1917,7 +1917,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
if (OwnsDefaultArg)
Record.AddTemplateArgumentLoc(D->getDefaultArgument());
- if (!TC && !OwnsDefaultArg &&
+ if (!D->hasTypeConstraint() && !OwnsDefaultArg &&
D->getDeclContext() == D->getLexicalDeclContext() &&
!D->isInvalidDecl() && !D->hasAttrs() &&
!D->isTopLevelDeclInObjCContainer() && !D->isImplicit() &&
@@ -2580,6 +2580,7 @@ void ASTWriter::WriteDeclAbbrevs() {
// TemplateTypeParmDecl
Abv->Add(
BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // wasDeclaredWithTypename
+ Abv->Add(BitCodeAbbrevOp(0));// TypeConstraintInitialized
Abv->Add(BitCodeAbbrevOp(0));// OwnsDefaultArg
DeclTemplateTypeParmAbbrev = Stream.EmitAbbrev(std::move(Abv));
diff --git a/clang/test/PCH/cxx2a-constraints-crash.cpp
b/clang/test/PCH/cxx2a-constraints-crash.cpp
index 637c55f0c879c9..0527c572e790b3 100644
--- a/clang/test/PCH/cxx2a-constraints-crash.cpp
+++ b/clang/test/PCH/cxx2a-constraints-crash.cpp
@@ -1,7 +1,5 @@
-// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t
-// RUN: %clang_cc1 -std=c++2a -include-pch %t -verify %s
-
-// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c++2a -fallow-pch-with-compiler-errors -emit-pch %s -o
%t -verify
+// RUN: %clang_cc1 -std=c++2a -fallow-pch-with-compiler-errors -include-pch %t
%s -verify
#ifndef HEADER
#define HEADER
@@ -27,3 +25,15 @@ int main() {
}
#endif
+
+namespace GH99036 {
+
+template
+concept C;
+// expected-error@-1 {{expected '='}} \
+// expected-note@-1 {{declared here}}
+
+template void f();
+// expected-error@-1 {{a concept definition cannot refer to itself}}
+
+} // namespace GH99036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Handle uninitialized type constraints (PR #110496)
falbrechtskirchinger wrote: > Bump, it would be nice to have this or that fix backported to 19.x @vient Backport is happening. #113182 https://github.com/llvm/llvm-project/pull/110496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Concepts] Add regression test for #99036 (PR #113137)
https://github.com/falbrechtskirchinger created
https://github.com/llvm/llvm-project/pull/113137
Add a test case for the assertion error encountered in #99036 and #109354. The
issue was incidentally fixed by #79.
>From a19b65b8512b103afae697cadf1ccd7cce4ef940 Mon Sep 17 00:00:00 2001
From: Florian Albrechtskirchinger
Date: Mon, 21 Oct 2024 11:18:29 +0200
Subject: [PATCH] [Concepts] Add regression test for #99036
Add a test case for the assertion error encountered in #99036 and
#109354. The issue was incidentally fixed by #79.
---
clang/test/PCH/cxx2a-constraints-crash.cpp | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/clang/test/PCH/cxx2a-constraints-crash.cpp
b/clang/test/PCH/cxx2a-constraints-crash.cpp
index 637c55f0c879c9..5c54ba4c425465 100644
--- a/clang/test/PCH/cxx2a-constraints-crash.cpp
+++ b/clang/test/PCH/cxx2a-constraints-crash.cpp
@@ -1,7 +1,5 @@
-// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t
-// RUN: %clang_cc1 -std=c++2a -include-pch %t -verify %s
-
-// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c++2a -fallow-pch-with-compiler-errors -emit-pch -o %t
%s -verify
+// RUN: %clang_cc1 -std=c++2a -fallow-pch-with-compiler-errors -include-pch %t
%s -verify
#ifndef HEADER
#define HEADER
@@ -27,3 +25,12 @@ int main() {
}
#endif
+
+namespace GH99036 {
+
+template
+concept C; // expected-error {{expected '='}}
+
+template void f();
+
+} // namespace GH99036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Concepts] Add regression test for #99036 (PR #113137)
falbrechtskirchinger wrote: @cor3ntin @zyn0217, please review. This is just the test case from #110496. Thanks! https://github.com/llvm/llvm-project/pull/113137 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Handle uninitialized type constraints (PR #110496)
falbrechtskirchinger wrote: > I think the best past forward would be to close this PR, and to open a new > one targeting the 10.x branch, which a clear explanation that it's not needed > in trunk. would that work for you @falbrechtskirchinger ? Will do. https://github.com/llvm/llvm-project/pull/110496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Handle uninitialized type constraints (PR #110496)
https://github.com/falbrechtskirchinger closed https://github.com/llvm/llvm-project/pull/110496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] [AST] Handle uninitialized type constraints (PR #110496)
https://github.com/falbrechtskirchinger updated
https://github.com/llvm/llvm-project/pull/110496
>From 8913503b3fd5518201663395d91b7444232e9856 Mon Sep 17 00:00:00 2001
From: Florian Albrechtskirchinger
Date: Mon, 30 Sep 2024 12:58:15 +0200
Subject: [PATCH] [clangd][AST] Handle uninitialized type constraints
The ASTWriter currently assumes template type constraints to be
initialized ((bool)getTypeConstraint() == hasTypeConstraint()).
The attached test case presents a scenario where that is not the case.
This patch removes the assumption and adds another boolean to the
serialization, to explicitly encode whether the type constraint has been
initialized.
Fixes #99036.
Fixes #109354.
---
.../clangd/unittests/ASTTests.cpp | 20 +++
clang/lib/Serialization/ASTReaderDecl.cpp | 2 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 5 +++--
3 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/ASTTests.cpp
b/clang-tools-extra/clangd/unittests/ASTTests.cpp
index 32c8e8a63a215a..6bf0667b68f967 100644
--- a/clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -658,6 +658,26 @@ TEST(ClangdAST, PreferredIncludeDirective) {
Symbol::IncludeDirective::Import);
}
+TEST(ClangdAST, HandleUninitializedTypeConstraints) {
+ auto TU = TestTU::withHeaderCode(R"cpp(
+template
+concept C; // error-ok
+template
+void f();
+)cpp");
+ TU.ExtraArgs.push_back("-std=c++20");
+
+ auto AST = TU.build();
+
+ const auto &F = llvm::cast(findDecl(AST, "f"));
+ const auto *Params = F.getTemplateParameters();
+ const auto *U = llvm::cast(Params->getParam(0));
+ const auto *TC = U->getTypeConstraint();
+
+ EXPECT_TRUE(U->hasTypeConstraint());
+ EXPECT_FALSE(/*TypeConstraintInitialized=*/TC != nullptr);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 7cead2728ca938..90783963934bb0 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2674,7 +2674,7 @@ void
ASTDeclReader::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
D->setDeclaredWithTypename(Record.readInt());
- if (D->hasTypeConstraint()) {
+ if (Record.readBool() && D->hasTypeConstraint()) {
ConceptReference *CR = nullptr;
if (Record.readBool())
CR = Record.readConceptReference();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index b71684569609ac..4da2108f553127 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1899,7 +1899,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
Record.push_back(D->wasDeclaredWithTypename());
const TypeConstraint *TC = D->getTypeConstraint();
- assert((bool)TC == D->hasTypeConstraint());
+ Record.push_back(/*TypeConstraintInitialized=*/TC != nullptr);
if (TC) {
auto *CR = TC->getConceptReference();
Record.push_back(CR != nullptr);
@@ -1917,7 +1917,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
if (OwnsDefaultArg)
Record.AddTemplateArgumentLoc(D->getDefaultArgument());
- if (!TC && !OwnsDefaultArg &&
+ if (!D->hasTypeConstraint() && !OwnsDefaultArg &&
D->getDeclContext() == D->getLexicalDeclContext() &&
!D->isInvalidDecl() && !D->hasAttrs() &&
!D->isTopLevelDeclInObjCContainer() && !D->isImplicit() &&
@@ -2580,6 +2580,7 @@ void ASTWriter::WriteDeclAbbrevs() {
// TemplateTypeParmDecl
Abv->Add(
BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // wasDeclaredWithTypename
+ Abv->Add(BitCodeAbbrevOp(0));// TypeConstraintInitialized
Abv->Add(BitCodeAbbrevOp(0));// OwnsDefaultArg
DeclTemplateTypeParmAbbrev = Stream.EmitAbbrev(std::move(Abv));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] [AST] Handle uninitialized type constraints (PR #110496)
falbrechtskirchinger wrote: > Thanks again for analyzing & working on this issue. You’re very welcome! Let's hope I'm on the right track. > Some suggestions regarding the test: > > 1. As this is more of a clang issue (the offending code belongs to the > serialization part of clang), so it is more appropriate to write a clang test > instead of a clangd test. > > 2. We don't usually write lit tests but rather unit tests (specifically, > tests under the `clangd/unittests` directory) for clangd. The most recent push contains a clangd unit test. I'd argue that replicating the test logic present in clangd (`TestTU`, `ParsedAST`) in clang unit tests is unnecessary. Also, the error is specific to clangd. No one has proposed a method of triggering the assertion using clang directly. I've verified that the test triggers the assertion without the changes introduced in this PR. ```txt [ RUN ] ClangdAST.HandleUninitializedTypeConstraints ClangdTests: /data6/git/llvm-project/clang/lib/Serialization/ASTWriterDecl.cpp:1902: void clang::ASTDeclWriter::VisitTemplateTypeParmDecl(clang::TemplateTypeParmDecl*): Assertion `(bool)TC == D->hasTypeConstraint()' failed. ``` If you insist on a clang unit test, I'll need help. :) https://github.com/llvm/llvm-project/pull/110496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] [AST] Handle uninitialized type constraints (PR #110496)
https://github.com/falbrechtskirchinger edited https://github.com/llvm/llvm-project/pull/110496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] [AST] Handle uninitialized type constraints (PR #110496)
https://github.com/falbrechtskirchinger created
https://github.com/llvm/llvm-project/pull/110496
Note: This patch is based on the assumption, that an assertion causing a crash
in clangd, is incorrect. Alternatively, the true error may lie elsewhere,
possibly in ´Sema::BuildTypeConstraint`/`Sema::AttachTypeConstraint`. Someone
with more experience is needed to make that call.
Commit message follows.
The ASTWriter currently assumes template type constraints to be initialized
((bool)getTypeConstraint() == hasTypeConstraint()). The attached test case
presents a scenario where that is not the case.
This patch removes the assumption and adds another boolean to the
serialization, to explicitly encode whether the type constraint has been
initialized.
Fixes #99036.
Fixes #109354.
>From 296cc1b1389cc1a7aff4f5a84a48b6db4fe9e59a Mon Sep 17 00:00:00 2001
From: Florian Albrechtskirchinger
Date: Mon, 30 Sep 2024 12:58:15 +0200
Subject: [PATCH] [clangd] [AST] Handle uninitialized type constraints
The ASTWriter currently assumes template type constraints to be
initialized ((bool)getTypeConstraint() == hasTypeConstraint()).
The attached test case presents a scenario where that is not the case.
This patch removes the assumption and adds another boolean to the
serialization, to explicitly encode whether the type constraint has been
initialized.
Fixes #99036.
Fixes #109354.
---
clang-tools-extra/clangd/test/GH99036.test | 29 ++
clang/lib/Serialization/ASTReaderDecl.cpp | 2 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 5 ++--
3 files changed, 33 insertions(+), 3 deletions(-)
create mode 100644 clang-tools-extra/clangd/test/GH99036.test
diff --git a/clang-tools-extra/clangd/test/GH99036.test
b/clang-tools-extra/clangd/test/GH99036.test
new file mode 100644
index 00..64ebe4c75a999d
--- /dev/null
+++ b/clang-tools-extra/clangd/test/GH99036.test
@@ -0,0 +1,29 @@
+# RUN: rm -rf %t
+# RUN: mkdir -p %t
+# RUN: split-file %s %t
+#
+# RUN: not clangd --check=%t/main.cpp 2>&1 | FileCheck -strict-whitespace %s
+#
+# CHECK: Loaded compilation database
+# CHECK-NEXT: Compile command inferred from main.cpp is:
+# CHECK: Indexing headers...
+# CHECK-NEXT: [unknown_typename] Line 1: in included file: unknown type name
'_Up'
+# CHECK: All checks completed, 1 errors
+
+#--- header.h
+template<_Up>
+concept __decayed_same_as;
+template<__decayed_same_as>
+partial_ordering operator0
+
+#--- main.cpp
+#include "header.h"
+
+#--- compile_commands.json
+[
+ {
+"directory": "/",
+"command": "c++ -std=c++20",
+"file": "main.cpp"
+ }
+]
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 7cead2728ca938..90783963934bb0 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2674,7 +2674,7 @@ void
ASTDeclReader::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
D->setDeclaredWithTypename(Record.readInt());
- if (D->hasTypeConstraint()) {
+ if (Record.readBool() && D->hasTypeConstraint()) {
ConceptReference *CR = nullptr;
if (Record.readBool())
CR = Record.readConceptReference();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index b71684569609ac..1c9898c042cd4b 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1899,7 +1899,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
Record.push_back(D->wasDeclaredWithTypename());
const TypeConstraint *TC = D->getTypeConstraint();
- assert((bool)TC == D->hasTypeConstraint());
+ Record.push_back(TC != nullptr); // reflects TypeConstraintInitialized
if (TC) {
auto *CR = TC->getConceptReference();
Record.push_back(CR != nullptr);
@@ -1917,7 +1917,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
if (OwnsDefaultArg)
Record.AddTemplateArgumentLoc(D->getDefaultArgument());
- if (!TC && !OwnsDefaultArg &&
+ if (!D->hasTypeConstraint() && !OwnsDefaultArg &&
D->getDeclContext() == D->getLexicalDeclContext() &&
!D->isInvalidDecl() && !D->hasAttrs() &&
!D->isTopLevelDeclInObjCContainer() && !D->isImplicit() &&
@@ -2580,6 +2580,7 @@ void ASTWriter::WriteDeclAbbrevs() {
// TemplateTypeParmDecl
Abv->Add(
BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // wasDeclaredWithTypename
+ Abv->Add(BitCodeAbbrevOp(0));// TypeConstraintInitialized
Abv->Add(BitCodeAbbrevOp(0));// OwnsDefaultArg
DeclTemplateTypeParmAbbrev = Stream.EmitAbbrev(std::move(Abv));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] [AST] Handle uninitialized type constraints (PR #110496)
@@ -1899,7 +1899,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
Record.push_back(D->wasDeclaredWithTypename());
const TypeConstraint *TC = D->getTypeConstraint();
- assert((bool)TC == D->hasTypeConstraint());
+ Record.push_back(TC != nullptr); // reflects TypeConstraintInitialized
falbrechtskirchinger wrote:
Done.
https://github.com/llvm/llvm-project/pull/110496
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] [AST] Handle uninitialized type constraints (PR #110496)
https://github.com/falbrechtskirchinger updated
https://github.com/llvm/llvm-project/pull/110496
>From 20042cc1b4ada2065813e118c24236a61e83d1a9 Mon Sep 17 00:00:00 2001
From: Florian Albrechtskirchinger
Date: Mon, 30 Sep 2024 12:58:15 +0200
Subject: [PATCH] [clangd] [AST] Handle uninitialized type constraints
The ASTWriter currently assumes template type constraints to be
initialized ((bool)getTypeConstraint() == hasTypeConstraint()).
The attached test case presents a scenario where that is not the case.
This patch removes the assumption and adds another boolean to the
serialization, to explicitly encode whether the type constraint has been
initialized.
Fixes #99036.
Fixes #109354.
---
clang-tools-extra/clangd/test/GH99036.test | 30 ++
clang/lib/Serialization/ASTReaderDecl.cpp | 2 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 5 ++--
3 files changed, 34 insertions(+), 3 deletions(-)
create mode 100644 clang-tools-extra/clangd/test/GH99036.test
diff --git a/clang-tools-extra/clangd/test/GH99036.test
b/clang-tools-extra/clangd/test/GH99036.test
new file mode 100644
index 00..6561d9d40ba5b5
--- /dev/null
+++ b/clang-tools-extra/clangd/test/GH99036.test
@@ -0,0 +1,30 @@
+# RUN: rm -rf %t
+# RUN: mkdir -p %t
+# RUN: split-file %s %t
+# RUN: sed -i 's|CWD|%/t|' %t/compile_commands.json
+#
+# RUN: not clangd --check=%t/main.cpp 2>&1 | FileCheck -strict-whitespace %s
+#
+# CHECK: Loaded compilation database
+# CHECK-NEXT: Compile command from CDB is:
+# CHECK: Indexing headers...
+# CHECK-NEXT: [expected] Line 1: in included file: expected '='
+# CHECK: All checks completed, 1 errors
+
+#--- header.h
+template
+concept C;
+template
+void f();
+
+#--- main.cpp
+#include "header.h"
+
+#--- compile_commands.json
+[
+ {
+"directory": "CWD",
+"command": "c++ -std=c++20",
+"file": "main.cpp"
+ }
+]
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 7cead2728ca938..90783963934bb0 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2674,7 +2674,7 @@ void
ASTDeclReader::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
D->setDeclaredWithTypename(Record.readInt());
- if (D->hasTypeConstraint()) {
+ if (Record.readBool() && D->hasTypeConstraint()) {
ConceptReference *CR = nullptr;
if (Record.readBool())
CR = Record.readConceptReference();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index b71684569609ac..1c9898c042cd4b 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1899,7 +1899,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
Record.push_back(D->wasDeclaredWithTypename());
const TypeConstraint *TC = D->getTypeConstraint();
- assert((bool)TC == D->hasTypeConstraint());
+ Record.push_back(TC != nullptr); // reflects TypeConstraintInitialized
if (TC) {
auto *CR = TC->getConceptReference();
Record.push_back(CR != nullptr);
@@ -1917,7 +1917,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
if (OwnsDefaultArg)
Record.AddTemplateArgumentLoc(D->getDefaultArgument());
- if (!TC && !OwnsDefaultArg &&
+ if (!D->hasTypeConstraint() && !OwnsDefaultArg &&
D->getDeclContext() == D->getLexicalDeclContext() &&
!D->isInvalidDecl() && !D->hasAttrs() &&
!D->isTopLevelDeclInObjCContainer() && !D->isImplicit() &&
@@ -2580,6 +2580,7 @@ void ASTWriter::WriteDeclAbbrevs() {
// TemplateTypeParmDecl
Abv->Add(
BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // wasDeclaredWithTypename
+ Abv->Add(BitCodeAbbrevOp(0));// TypeConstraintInitialized
Abv->Add(BitCodeAbbrevOp(0));// OwnsDefaultArg
DeclTemplateTypeParmAbbrev = Stream.EmitAbbrev(std::move(Abv));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clangd] [AST] Handle uninitialized type constraints (PR #110496)
https://github.com/falbrechtskirchinger updated
https://github.com/llvm/llvm-project/pull/110496
>From c090601a032c6ba406928821ec4296d190edade5 Mon Sep 17 00:00:00 2001
From: Florian Albrechtskirchinger
Date: Mon, 30 Sep 2024 12:58:15 +0200
Subject: [PATCH] [clangd][AST] Handle uninitialized type constraints
The ASTWriter currently assumes template type constraints to be
initialized ((bool)getTypeConstraint() == hasTypeConstraint()).
The attached test case presents a scenario where that is not the case.
This patch removes the assumption and adds another boolean to the
serialization, to explicitly encode whether the type constraint has been
initialized.
Fixes #99036.
Fixes #109354.
---
clang/lib/Serialization/ASTReaderDecl.cpp | 2 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 5 +++--
clang/test/PCH/cxx2a-constraints-crash.cpp | 15 +++
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 7cead2728ca938..90783963934bb0 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2674,7 +2674,7 @@ void
ASTDeclReader::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
D->setDeclaredWithTypename(Record.readInt());
- if (D->hasTypeConstraint()) {
+ if (Record.readBool() && D->hasTypeConstraint()) {
ConceptReference *CR = nullptr;
if (Record.readBool())
CR = Record.readConceptReference();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index b71684569609ac..4da2108f553127 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1899,7 +1899,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
Record.push_back(D->wasDeclaredWithTypename());
const TypeConstraint *TC = D->getTypeConstraint();
- assert((bool)TC == D->hasTypeConstraint());
+ Record.push_back(/*TypeConstraintInitialized=*/TC != nullptr);
if (TC) {
auto *CR = TC->getConceptReference();
Record.push_back(CR != nullptr);
@@ -1917,7 +1917,7 @@ void
ASTDeclWriter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
if (OwnsDefaultArg)
Record.AddTemplateArgumentLoc(D->getDefaultArgument());
- if (!TC && !OwnsDefaultArg &&
+ if (!D->hasTypeConstraint() && !OwnsDefaultArg &&
D->getDeclContext() == D->getLexicalDeclContext() &&
!D->isInvalidDecl() && !D->hasAttrs() &&
!D->isTopLevelDeclInObjCContainer() && !D->isImplicit() &&
@@ -2580,6 +2580,7 @@ void ASTWriter::WriteDeclAbbrevs() {
// TemplateTypeParmDecl
Abv->Add(
BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // wasDeclaredWithTypename
+ Abv->Add(BitCodeAbbrevOp(0));// TypeConstraintInitialized
Abv->Add(BitCodeAbbrevOp(0));// OwnsDefaultArg
DeclTemplateTypeParmAbbrev = Stream.EmitAbbrev(std::move(Abv));
diff --git a/clang/test/PCH/cxx2a-constraints-crash.cpp
b/clang/test/PCH/cxx2a-constraints-crash.cpp
index 637c55f0c879c9..4a74740b918437 100644
--- a/clang/test/PCH/cxx2a-constraints-crash.cpp
+++ b/clang/test/PCH/cxx2a-constraints-crash.cpp
@@ -1,7 +1,5 @@
-// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t
-// RUN: %clang_cc1 -std=c++2a -include-pch %t -verify %s
-
-// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c++2a -fallow-pch-with-compiler-errors -emit-pch %s -o
%t -verify
+// RUN: %clang_cc1 -std=c++2a -fallow-pch-with-compiler-errors -include-pch %t
%s -verify
#ifndef HEADER
#define HEADER
@@ -27,3 +25,12 @@ int main() {
}
#endif
+
+namespace GH99036 {
+
+template
+concept C; // expected-error {{expected '='}}
+// expected-note@32 {{declared here}}
+template void f(); // expected-error {{a concept definition cannot refer
to itself}}
+
+} // namespace GH99036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] [AST] Handle uninitialized type constraints (PR #110496)
falbrechtskirchinger wrote: > We need to resort to `-fallow-pch-with-compiler-errors` :) Ah, thanks! I would not have figured that out on my own. :) > Specifically, add the following file to `clang/test/PCH` (or somehow merge it > into a pre-existing test, which I'd prefer) I looked through that directory and found only one test with `-fallow-pch-with-compiler-errors` and `-std=c++20`: `race-condition.cpp` – not a great fit. `cxx2a-constraints-crash.cpp` sounds promising and doesn't need too much modification. I'm including this in my next push, but I have an annotated standalone file based on your suggestion ready to go if needed. https://github.com/llvm/llvm-project/pull/110496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clangd] [AST] Handle uninitialized type constraints (PR #110496)
@@ -27,3 +25,12 @@ int main() {
}
#endif
+
+namespace GH99036 {
+
+template
+concept C; // expected-error {{expected '='}}
+// expected-note@32 {{declared here}}
falbrechtskirchinger wrote:
Should have used `@-1` here.
https://github.com/llvm/llvm-project/pull/110496
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
