[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG50b1faf5c188: [Clang] CWG 1394: Incomplete types as
parameters of deleted functions (authored by rZhBoYao).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122981/new/
https://reviews.llvm.org/D122981
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
clang/www/cxx_dr_status.html
Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -8178,7 +8178,7 @@
https://wg21.link/cwg1394;>1394
CD3
Incomplete types as parameters of deleted functions
-Unknown
+Clang 15
https://wg21.link/cwg1395;>1395
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Incomplete; // expected-note 2{{forward declaration of 'Incomplete'}}
+Incomplete f(Incomplete) = delete; // well-formed
+Incomplete g(Incomplete) {}// expected-error{{incomplete result type 'Incomplete' in function definition}}\
+// expected-error{{variable has incomplete type 'Incomplete'}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17379,6 +17379,21 @@
}
}
+void Sema::SetFunctionBodyKind(Decl *D, SourceLocation Loc,
+ FnBodyKind BodyKind) {
+ switch (BodyKind) {
+ case FnBodyKind::Delete:
+SetDeclDeleted(D, Loc);
+break;
+ case FnBodyKind::Default:
+SetDeclDefaulted(D, Loc);
+break;
+ case FnBodyKind::Other:
+llvm_unreachable(
+"Parsed function body should be '= delete;' or '= default;'");
+ }
+}
+
bool Sema::CheckOverridingFunctionAttributes(const CXXMethodDecl *New,
const CXXMethodDecl *Old) {
const auto *NewFT = New->getType()->castAs();
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14271,7 +14271,7 @@
Decl *
Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Declarator ,
MultiTemplateParamsArg TemplateParameterLists,
- SkipBodyInfo *SkipBody) {
+ SkipBodyInfo *SkipBody, FnBodyKind BodyKind) {
assert(getCurFunctionDecl() == nullptr && "Function parsing confused");
assert(D.isFunctionDeclarator() && "Not a function declarator!");
Scope *ParentScope = FnBodyScope->getParent();
@@ -14290,7 +14290,7 @@
D.setFunctionDefinitionKind(FunctionDefinitionKind::Definition);
Decl *DP = HandleDeclarator(ParentScope, D, TemplateParameterLists);
- Decl *Dcl = ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);
+ Decl *Dcl = ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody, BodyKind);
if (!Bases.empty())
ActOnFinishedFunctionDefinitionInOpenMPDeclareVariantScope(Dcl, Bases);
@@ -14466,7 +14466,8 @@
}
Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
-SkipBodyInfo *SkipBody) {
+SkipBodyInfo *SkipBody,
+FnBodyKind BodyKind) {
if (!D) {
// Parsing the function declaration failed in some way. Push on a fake scope
// anyway so we can try to parse the function body.
@@ -14555,11 +14556,11 @@
}
}
- // The return type of a function definition must be complete
- // (C99 6.9.1p3, C++ [dcl.fct]p6).
+ // The return type of a function definition must be complete (C99 6.9.1p3),
+ // unless the function is deleted (C++ specifc, C++ [dcl.fct.def.general]p2)
QualType ResultType = FD->getReturnType();
if (!ResultType->isDependentType() && !ResultType->isVoidType() &&
- !FD->isInvalidDecl() &&
+ !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
RequireCompleteType(FD->getLocation(), ResultType,
diag::err_func_def_incomplete_result))
FD->setInvalidDecl();
@@ -14568,8 +14569,9 @@
PushDeclContext(FnBodyScope, FD);
// Check the validity of our function parameters
- CheckParmsForFunctionDef(FD->parameters(),
- /*CheckParameterNames=*/true);
+ if (BodyKind != FnBodyKind::Delete)
+CheckParmsForFunctionDef(FD->parameters(),
+ /*CheckParameterNames=*/true);
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
ChuanqiXu accepted this revision. ChuanqiXu added a comment. LGTM then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122981/new/ https://reviews.llvm.org/D122981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
rZhBoYao added a comment. Revisiting @ChuanqiXu 's code suggestion... Comment at: clang/include/clang/Sema/Sema.h:2899-2909 +/// C++ [dcl.fct.def.general]p1 +/// function-body: +/// = delete ; +/// = default ; +Delete, +Default, + erichkeane wrote: > rZhBoYao wrote: > > ChuanqiXu wrote: > > > rZhBoYao wrote: > > > > ChuanqiXu wrote: > > > > > Agree to @erichkeane > > > > With all due respect, this code suggestion doesn't make any sense to > > > > me. My best guess is @ChuanqiXu was thinking the order specified by the > > > > grammar as noted in [[ > > > > https://eel.is/c++draft/dcl.fct.def.general#nt:function-body | > > > > dcl.fct.def.general p1 ]]. Even if that was the case, `CompoundStmt` is > > > > not quite right either. Also, differentiating `ctor-initializer[opt] > > > > compound-statement` and `function-try-block` is meaningless here, hence > > > > the name `Other`. > > > > > > > > I adopted the same order as to how `Parser::ParseFunctionDefinition` > > > > has always been parsing `function-body`. The order is not significant > > > > in any meaningful way as each of the 4 grammar productions of > > > > `function-body` is VERY different and mutually exclusive. Putting > > > > `Delete` and `Default` upfront not only emphasizes the "specialness" of > > > > them but also conveys how we handle `function-body`. > > > > > > > > What say you, @erichkeane ? > > > Yeah, the order comes from the standard. I think the comment should be > > > consistent with the spec. And for the name, I agree `CompoundStmt` is not > > > accurate indeed... I thought Default should be a good name but there is > > > `Default` already. But I don't feel `Other` is good since the > > > compound-statement is much more common. > > > > > > > Putting Delete and Default upfront not only emphasizes the > > > > "specialness" of them but also conveys how we handle function-body. > > > > > > This don't makes sense. Generally, we would put common items in front. > > > And the order here shouldn't be related to the implementation. > > > But I don't feel `Other` is good since the compound-statement is much > > > more common. > > `Other` reads "not special like `Delete` and `Default`". > > > This don't makes sense. Generally, we would put common items in front. > > > And the order here shouldn't be related to the implementation. > > Think of it as a not-so-special else clause at the end of an if/else chain. > > We tend to process special cases upfront. It is only natural. > > > > I'll await @erichkeane 's final verdict. > >>Think of it as a not-so-special else clause at the end of an if/else chain. > >>We tend to process special cases upfront. It is only natural. > > This was my understanding. This is a single value that means 'not default > and not deleted'. I think the idea of doing it in standards order is a > really good one, but have the top 2 just both result in `Other`, `Unknown`, > etc. So something like: > > ``` > //ctor-initializeropt compound-statement > //function-try-block > Unknown, > //= default ; > Default, > //= delete ; > Delete > ``` > > BUT as this is not an 'ast' level flag, it is just for the purposes of > simplifying this call, I don't think it needs to conform that closely. I actually don't know what you were trying to say here: > We don't yet part function-try-block yet. > FunctionTryBlock, especially: > We don't yet part function-try-block yet. Nonetheless, I did my best to cater to your comment within reason, such as reordering enumerators to align with the C++ standard text. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122981/new/ https://reviews.llvm.org/D122981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
rZhBoYao updated this revision to Diff 421589.
rZhBoYao marked 2 inline comments as done.
rZhBoYao set the repository for this revision to rG LLVM Github Monorepo.
rZhBoYao added a comment.
Reordered enumerators.
Does this look good to you, @ChuanqiXu ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122981/new/
https://reviews.llvm.org/D122981
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
clang/www/cxx_dr_status.html
Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -8178,7 +8178,7 @@
https://wg21.link/cwg1394;>1394
CD3
Incomplete types as parameters of deleted functions
-Unknown
+Clang 15
https://wg21.link/cwg1395;>1395
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Incomplete; // expected-note 2{{forward declaration of 'Incomplete'}}
+Incomplete f(Incomplete) = delete; // well-formed
+Incomplete g(Incomplete) {}// expected-error{{incomplete result type 'Incomplete' in function definition}}\
+// expected-error{{variable has incomplete type 'Incomplete'}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17379,6 +17379,21 @@
}
}
+void Sema::SetFunctionBodyKind(Decl *D, SourceLocation Loc,
+ FnBodyKind BodyKind) {
+ switch (BodyKind) {
+ case FnBodyKind::Delete:
+SetDeclDeleted(D, Loc);
+break;
+ case FnBodyKind::Default:
+SetDeclDefaulted(D, Loc);
+break;
+ case FnBodyKind::Other:
+llvm_unreachable(
+"Parsed function body should be '= delete;' or '= default;'");
+ }
+}
+
bool Sema::CheckOverridingFunctionAttributes(const CXXMethodDecl *New,
const CXXMethodDecl *Old) {
const auto *NewFT = New->getType()->castAs();
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14191,7 +14191,7 @@
Decl *
Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Declarator ,
MultiTemplateParamsArg TemplateParameterLists,
- SkipBodyInfo *SkipBody) {
+ SkipBodyInfo *SkipBody, FnBodyKind BodyKind) {
assert(getCurFunctionDecl() == nullptr && "Function parsing confused");
assert(D.isFunctionDeclarator() && "Not a function declarator!");
Scope *ParentScope = FnBodyScope->getParent();
@@ -14210,7 +14210,7 @@
D.setFunctionDefinitionKind(FunctionDefinitionKind::Definition);
Decl *DP = HandleDeclarator(ParentScope, D, TemplateParameterLists);
- Decl *Dcl = ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);
+ Decl *Dcl = ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody, BodyKind);
if (!Bases.empty())
ActOnFinishedFunctionDefinitionInOpenMPDeclareVariantScope(Dcl, Bases);
@@ -14386,7 +14386,8 @@
}
Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
-SkipBodyInfo *SkipBody) {
+SkipBodyInfo *SkipBody,
+FnBodyKind BodyKind) {
if (!D) {
// Parsing the function declaration failed in some way. Push on a fake scope
// anyway so we can try to parse the function body.
@@ -14475,11 +14476,11 @@
}
}
- // The return type of a function definition must be complete
- // (C99 6.9.1p3, C++ [dcl.fct]p6).
+ // The return type of a function definition must be complete (C99 6.9.1p3),
+ // unless the function is deleted (C++ specifc, C++ [dcl.fct.def.general]p2)
QualType ResultType = FD->getReturnType();
if (!ResultType->isDependentType() && !ResultType->isVoidType() &&
- !FD->isInvalidDecl() &&
+ !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
RequireCompleteType(FD->getLocation(), ResultType,
diag::err_func_def_incomplete_result))
FD->setInvalidDecl();
@@ -14488,8 +14489,9 @@
PushDeclContext(FnBodyScope, FD);
// Check the validity of our function parameters
- CheckParmsForFunctionDef(FD->parameters(),
- /*CheckParameterNames=*/true);
+ if (BodyKind != FnBodyKind::Delete)
+CheckParmsForFunctionDef(FD->parameters(),
+ /*CheckParameterNames=*/true);
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. I'm going to accept, I am ok with it as-is, though I would like to see @ChuanqiXu be accepting of whatever we do in Sema's enum as well. So please wait for his feedback too. Comment at: clang/include/clang/Sema/Sema.h:2899-2909 +/// C++ [dcl.fct.def.general]p1 +/// function-body: +/// = delete ; +/// = default ; +Delete, +Default, + rZhBoYao wrote: > ChuanqiXu wrote: > > rZhBoYao wrote: > > > ChuanqiXu wrote: > > > > Agree to @erichkeane > > > With all due respect, this code suggestion doesn't make any sense to me. > > > My best guess is @ChuanqiXu was thinking the order specified by the > > > grammar as noted in [[ > > > https://eel.is/c++draft/dcl.fct.def.general#nt:function-body | > > > dcl.fct.def.general p1 ]]. Even if that was the case, `CompoundStmt` is > > > not quite right either. Also, differentiating `ctor-initializer[opt] > > > compound-statement` and `function-try-block` is meaningless here, hence > > > the name `Other`. > > > > > > I adopted the same order as to how `Parser::ParseFunctionDefinition` has > > > always been parsing `function-body`. The order is not significant in any > > > meaningful way as each of the 4 grammar productions of `function-body` is > > > VERY different and mutually exclusive. Putting `Delete` and `Default` > > > upfront not only emphasizes the "specialness" of them but also conveys > > > how we handle `function-body`. > > > > > > What say you, @erichkeane ? > > Yeah, the order comes from the standard. I think the comment should be > > consistent with the spec. And for the name, I agree `CompoundStmt` is not > > accurate indeed... I thought Default should be a good name but there is > > `Default` already. But I don't feel `Other` is good since the > > compound-statement is much more common. > > > > > Putting Delete and Default upfront not only emphasizes the "specialness" > > > of them but also conveys how we handle function-body. > > > > This don't makes sense. Generally, we would put common items in front. And > > the order here shouldn't be related to the implementation. > > But I don't feel `Other` is good since the compound-statement is much more > > common. > `Other` reads "not special like `Delete` and `Default`". > > This don't makes sense. Generally, we would put common items in front. And > > the order here shouldn't be related to the implementation. > Think of it as a not-so-special else clause at the end of an if/else chain. > We tend to process special cases upfront. It is only natural. > > I'll await @erichkeane 's final verdict. >>Think of it as a not-so-special else clause at the end of an if/else chain. >>We tend to process special cases upfront. It is only natural. This was my understanding. This is a single value that means 'not default and not deleted'. I think the idea of doing it in standards order is a really good one, but have the top 2 just both result in `Other`, `Unknown`, etc. So something like: ``` //ctor-initializeropt compound-statement //function-try-block Unknown, //= default ; Default, //= delete ; Delete ``` BUT as this is not an 'ast' level flag, it is just for the purposes of simplifying this call, I don't think it needs to conform that closely. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122981/new/ https://reviews.llvm.org/D122981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
rZhBoYao marked 2 inline comments as done. rZhBoYao added inline comments. Comment at: clang/include/clang/Sema/Sema.h:2899-2909 +/// C++ [dcl.fct.def.general]p1 +/// function-body: +/// = delete ; +/// = default ; +Delete, +Default, + ChuanqiXu wrote: > rZhBoYao wrote: > > ChuanqiXu wrote: > > > Agree to @erichkeane > > With all due respect, this code suggestion doesn't make any sense to me. My > > best guess is @ChuanqiXu was thinking the order specified by the grammar as > > noted in [[ https://eel.is/c++draft/dcl.fct.def.general#nt:function-body | > > dcl.fct.def.general p1 ]]. Even if that was the case, `CompoundStmt` is not > > quite right either. Also, differentiating `ctor-initializer[opt] > > compound-statement` and `function-try-block` is meaningless here, hence the > > name `Other`. > > > > I adopted the same order as to how `Parser::ParseFunctionDefinition` has > > always been parsing `function-body`. The order is not significant in any > > meaningful way as each of the 4 grammar productions of `function-body` is > > VERY different and mutually exclusive. Putting `Delete` and `Default` > > upfront not only emphasizes the "specialness" of them but also conveys how > > we handle `function-body`. > > > > What say you, @erichkeane ? > Yeah, the order comes from the standard. I think the comment should be > consistent with the spec. And for the name, I agree `CompoundStmt` is not > accurate indeed... I thought Default should be a good name but there is > `Default` already. But I don't feel `Other` is good since the > compound-statement is much more common. > > > Putting Delete and Default upfront not only emphasizes the "specialness" of > > them but also conveys how we handle function-body. > > This don't makes sense. Generally, we would put common items in front. And > the order here shouldn't be related to the implementation. > But I don't feel `Other` is good since the compound-statement is much more > common. `Other` reads "not special like `Delete` and `Default`". > This don't makes sense. Generally, we would put common items in front. And > the order here shouldn't be related to the implementation. Think of it as a not-so-special else clause at the end of an if/else chain. We tend to process special cases upfront. It is only natural. I'll await @erichkeane 's final verdict. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122981/new/ https://reviews.llvm.org/D122981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
ChuanqiXu added inline comments. Comment at: clang/include/clang/Sema/Sema.h:2899-2909 +/// C++ [dcl.fct.def.general]p1 +/// function-body: +/// = delete ; +/// = default ; +Delete, +Default, + rZhBoYao wrote: > ChuanqiXu wrote: > > Agree to @erichkeane > With all due respect, this code suggestion doesn't make any sense to me. My > best guess is @ChuanqiXu was thinking the order specified by the grammar as > noted in [[ https://eel.is/c++draft/dcl.fct.def.general#nt:function-body | > dcl.fct.def.general p1 ]]. Even if that was the case, `CompoundStmt` is not > quite right either. Also, differentiating `ctor-initializer[opt] > compound-statement` and `function-try-block` is meaningless here, hence the > name `Other`. > > I adopted the same order as to how `Parser::ParseFunctionDefinition` has > always been parsing `function-body`. The order is not significant in any > meaningful way as each of the 4 grammar productions of `function-body` is > VERY different and mutually exclusive. Putting `Delete` and `Default` upfront > not only emphasizes the "specialness" of them but also conveys how we handle > `function-body`. > > What say you, @erichkeane ? Yeah, the order comes from the standard. I think the comment should be consistent with the spec. And for the name, I agree `CompoundStmt` is not accurate indeed... I thought Default should be a good name but there is `Default` already. But I don't feel `Other` is good since the compound-statement is much more common. > Putting Delete and Default upfront not only emphasizes the "specialness" of > them but also conveys how we handle function-body. This don't makes sense. Generally, we would put common items in front. And the order here shouldn't be related to the implementation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122981/new/ https://reviews.llvm.org/D122981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
rZhBoYao marked an inline comment as done. rZhBoYao added inline comments. Comment at: clang/include/clang/Sema/Sema.h:2899-2909 +/// C++ [dcl.fct.def.general]p1 +/// function-body: +/// = delete ; +/// = default ; +Delete, +Default, + ChuanqiXu wrote: > Agree to @erichkeane With all due respect, this code suggestion doesn't make any sense to me. My best guess is @ChuanqiXu was thinking the order specified by the grammar as noted in [[ https://eel.is/c++draft/dcl.fct.def.general#nt:function-body | dcl.fct.def.general p1 ]]. Even if that was the case, `CompoundStmt` is not quite right either. Also, differentiating `ctor-initializer[opt] compound-statement` and `function-try-block` is meaningless here, hence the name `Other`. I adopted the same order as to how `Parser::ParseFunctionDefinition` has always been parsing `function-body`. The order is not significant in any meaningful way as each of the 4 grammar productions of `function-body` is VERY different and mutually exclusive. Putting `Delete` and `Default` upfront not only emphasizes the "specialness" of them but also conveys how we handle `function-body`. What say you, @erichkeane ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122981/new/ https://reviews.llvm.org/D122981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
ChuanqiXu added a comment. I agree this one is better. Comment at: clang/include/clang/Sema/Sema.h:2899-2909 +/// C++ [dcl.fct.def.general]p1 +/// function-body: +/// = delete ; +/// = default ; +Delete, +Default, + Agree to @erichkeane CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122981/new/ https://reviews.llvm.org/D122981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
rZhBoYao updated this revision to Diff 421345.
rZhBoYao added a comment.
Added release note.
Also, push to my repo to make sure the release note is correctly rendered:
https://github.com/poyaoc97/llvm-project/commit/1167f0fc6b91
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122981/new/
https://reviews.llvm.org/D122981
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
clang/www/cxx_dr_status.html
Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -8178,7 +8178,7 @@
https://wg21.link/cwg1394;>1394
CD3
Incomplete types as parameters of deleted functions
-Unknown
+Clang 15
https://wg21.link/cwg1395;>1395
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Incomplete; // expected-note 2{{forward declaration of 'Incomplete'}}
+Incomplete f(Incomplete) = delete; // well-formed
+Incomplete g(Incomplete) {}// expected-error{{incomplete result type 'Incomplete' in function definition}}\
+// expected-error{{variable has incomplete type 'Incomplete'}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17379,6 +17379,21 @@
}
}
+void Sema::SetFunctionBodyKind(Decl *D, SourceLocation Loc,
+ FnBodyKind BodyKind) {
+ switch (BodyKind) {
+ case FnBodyKind::Delete:
+SetDeclDeleted(D, Loc);
+break;
+ case FnBodyKind::Default:
+SetDeclDefaulted(D, Loc);
+break;
+ case FnBodyKind::Other:
+llvm_unreachable(
+"Parsed function body should be '= delete;' or '= default;'");
+ }
+}
+
bool Sema::CheckOverridingFunctionAttributes(const CXXMethodDecl *New,
const CXXMethodDecl *Old) {
const auto *NewFT = New->getType()->castAs();
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14185,7 +14185,7 @@
Decl *
Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Declarator ,
MultiTemplateParamsArg TemplateParameterLists,
- SkipBodyInfo *SkipBody) {
+ SkipBodyInfo *SkipBody, FnBodyKind BodyKind) {
assert(getCurFunctionDecl() == nullptr && "Function parsing confused");
assert(D.isFunctionDeclarator() && "Not a function declarator!");
Scope *ParentScope = FnBodyScope->getParent();
@@ -14204,7 +14204,7 @@
D.setFunctionDefinitionKind(FunctionDefinitionKind::Definition);
Decl *DP = HandleDeclarator(ParentScope, D, TemplateParameterLists);
- Decl *Dcl = ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);
+ Decl *Dcl = ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody, BodyKind);
if (!Bases.empty())
ActOnFinishedFunctionDefinitionInOpenMPDeclareVariantScope(Dcl, Bases);
@@ -14380,7 +14380,8 @@
}
Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
-SkipBodyInfo *SkipBody) {
+SkipBodyInfo *SkipBody,
+FnBodyKind BodyKind) {
if (!D) {
// Parsing the function declaration failed in some way. Push on a fake scope
// anyway so we can try to parse the function body.
@@ -14469,11 +14470,11 @@
}
}
- // The return type of a function definition must be complete
- // (C99 6.9.1p3, C++ [dcl.fct]p6).
+ // The return type of a function definition must be complete (C99 6.9.1p3),
+ // unless the function is deleted (C++ specifc, C++ [dcl.fct.def.general]p2)
QualType ResultType = FD->getReturnType();
if (!ResultType->isDependentType() && !ResultType->isVoidType() &&
- !FD->isInvalidDecl() &&
+ !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
RequireCompleteType(FD->getLocation(), ResultType,
diag::err_func_def_incomplete_result))
FD->setInvalidDecl();
@@ -14482,8 +14483,9 @@
PushDeclContext(FnBodyScope, FD);
// Check the validity of our function parameters
- CheckParmsForFunctionDef(FD->parameters(),
- /*CheckParameterNames=*/true);
+ if (BodyKind != FnBodyKind::Delete)
+CheckParmsForFunctionDef(FD->parameters(),
+ /*CheckParameterNames=*/true);
// Add non-parameter declarations already in the function
[PATCH] D122981: [Clang] CWG 1394: Incomplete types as parameters of deleted functions
rZhBoYao updated this revision to Diff 421338.
rZhBoYao retitled this revision from "[Clang] Diagnose incomplete return/param
types only when function is not deleted" to "[Clang] CWG 1394: Incomplete types
as parameters of deleted functions".
rZhBoYao edited the summary of this revision.
rZhBoYao removed a reviewer: rtrieu.
rZhBoYao set the repository for this revision to rG LLVM Github Monorepo.
rZhBoYao added a comment.
Updating the status of CWG 1394 is the only change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122981/new/
https://reviews.llvm.org/D122981
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
clang/www/cxx_dr_status.html
Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -8178,7 +8178,7 @@
https://wg21.link/cwg1394;>1394
CD3
Incomplete types as parameters of deleted functions
-Unknown
+Clang 15
https://wg21.link/cwg1395;>1395
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.general/p2.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Incomplete; // expected-note 2{{forward declaration of 'Incomplete'}}
+Incomplete f(Incomplete) = delete; // well-formed
+Incomplete g(Incomplete) {}// expected-error{{incomplete result type 'Incomplete' in function definition}}\
+// expected-error{{variable has incomplete type 'Incomplete'}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17379,6 +17379,21 @@
}
}
+void Sema::SetFunctionBodyKind(Decl *D, SourceLocation Loc,
+ FnBodyKind BodyKind) {
+ switch (BodyKind) {
+ case FnBodyKind::Delete:
+SetDeclDeleted(D, Loc);
+break;
+ case FnBodyKind::Default:
+SetDeclDefaulted(D, Loc);
+break;
+ case FnBodyKind::Other:
+llvm_unreachable(
+"Parsed function body should be '= delete;' or '= default;'");
+ }
+}
+
bool Sema::CheckOverridingFunctionAttributes(const CXXMethodDecl *New,
const CXXMethodDecl *Old) {
const auto *NewFT = New->getType()->castAs();
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14185,7 +14185,7 @@
Decl *
Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Declarator ,
MultiTemplateParamsArg TemplateParameterLists,
- SkipBodyInfo *SkipBody) {
+ SkipBodyInfo *SkipBody, FnBodyKind BodyKind) {
assert(getCurFunctionDecl() == nullptr && "Function parsing confused");
assert(D.isFunctionDeclarator() && "Not a function declarator!");
Scope *ParentScope = FnBodyScope->getParent();
@@ -14204,7 +14204,7 @@
D.setFunctionDefinitionKind(FunctionDefinitionKind::Definition);
Decl *DP = HandleDeclarator(ParentScope, D, TemplateParameterLists);
- Decl *Dcl = ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);
+ Decl *Dcl = ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody, BodyKind);
if (!Bases.empty())
ActOnFinishedFunctionDefinitionInOpenMPDeclareVariantScope(Dcl, Bases);
@@ -14380,7 +14380,8 @@
}
Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
-SkipBodyInfo *SkipBody) {
+SkipBodyInfo *SkipBody,
+FnBodyKind BodyKind) {
if (!D) {
// Parsing the function declaration failed in some way. Push on a fake scope
// anyway so we can try to parse the function body.
@@ -14469,11 +14470,11 @@
}
}
- // The return type of a function definition must be complete
- // (C99 6.9.1p3, C++ [dcl.fct]p6).
+ // The return type of a function definition must be complete (C99 6.9.1p3),
+ // unless the function is deleted (C++ specifc, C++ [dcl.fct.def.general]p2)
QualType ResultType = FD->getReturnType();
if (!ResultType->isDependentType() && !ResultType->isVoidType() &&
- !FD->isInvalidDecl() &&
+ !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
RequireCompleteType(FD->getLocation(), ResultType,
diag::err_func_def_incomplete_result))
FD->setInvalidDecl();
@@ -14482,8 +14483,9 @@
PushDeclContext(FnBodyScope, FD);
// Check the validity of our function parameters
- CheckParmsForFunctionDef(FD->parameters(),
-
