-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[Clang] [C++26] Implement P2573R2: = delete("should have a reason");
#86526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang-modules Author: None (Sirraide) ChangesThis implements support for the The major parts are all done, from what I can tell, except for one thing: we need to figure out where to store the message. We’re out of bits in There are some fairly straight-forward things that are still missing; I’m putting these here so I don’t end up forgetting about any of them (I’ll work on these while we figure out where to put the message):
Patch is 24.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86526.diff 20 Files Affected:
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index a5879591f4c659..9c445eccdf8992 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2013,6 +2013,12 @@ class FunctionDecl : public DeclaratorDecl,
DefaultedFunctionInfo *DefaultedInfo;
};
+ /// Message that indicates why this function was deleted.
+ ///
+ /// FIXME: Figure out where to actually put this; maybe in the
+ /// 'DefaultedInfo' above?
+ StringLiteral *DeletedMessage;
+
unsigned ODRHash;
/// End part of this FunctionDecl's source range.
@@ -2483,6 +2489,10 @@ class FunctionDecl : public DeclaratorDecl,
}
void setDeletedAsWritten(bool D = true) { FunctionDeclBits.IsDeleted = D; }
+ void setDeletedWithMessage(StringLiteral *Message) {
+ FunctionDeclBits.IsDeleted = true;
+ DeletedMessage = Message;
+ }
/// Determines whether this function is "main", which is the
/// entry point into an executable program.
@@ -2638,6 +2648,9 @@ class FunctionDecl : public DeclaratorDecl,
AC.push_back(TRC);
}
+ /// Get the message that indicates why this function was deleted.
+ StringLiteral *getDeletedMessage() const { return DeletedMessage; }
+
void setPreviousDeclaration(FunctionDecl * PrevDecl);
FunctionDecl *getCanonicalDecl() override;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d7ab1635cf12bc..37197b60ddfbb8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4661,11 +4661,10 @@ def err_ovl_no_viable_member_function_in_call : Error<
"no matching member function for call to %0">;
def err_ovl_ambiguous_call : Error<
"call to %0 is ambiguous">;
-def err_ovl_deleted_call : Error<"call to deleted function %0">;
+def err_ovl_deleted_call : Error<"call to deleted"
+ "%select{| member}0 function %1%select{|: %3}2">;
def err_ovl_ambiguous_member_call : Error<
"call to member function %0 is ambiguous">;
-def err_ovl_deleted_member_call : Error<
- "call to deleted member function %0">;
def note_ovl_too_many_candidates : Note<
"remaining %0 candidate%s0 omitted; "
"pass -fshow-overloads=all to show them">;
@@ -8857,7 +8856,7 @@ def err_nontemporal_builtin_must_be_pointer_intfltptr_or_vector : Error<
"address argument to nontemporal builtin must be a pointer to integer, float, "
"pointer, or a vector of such types (%0 invalid)">;
-def err_deleted_function_use : Error<"attempt to use a deleted function">;
+def err_deleted_function_use : Error<"attempt to use a deleted function%select{|: %1}0">;
def err_deleted_inherited_ctor_use : Error<
"constructor inherited by %0 from base class %1 is implicitly deleted">;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 14df75180ef321..559e6416b7dfb3 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1600,6 +1600,7 @@ class Parser : public CodeCompletionHandler {
const ParsedTemplateInfo &TemplateInfo,
const VirtSpecifiers &VS,
SourceLocation PureSpecLoc);
+ StringLiteral *ParseCXXDeletedFunctionMessage();
void ParseCXXNonStaticMemberInitializer(Decl *VarD);
void ParseLexedAttributes(ParsingClass &Class);
void ParseLexedAttributeList(LateParsedAttrList &LAs, Decl *D,
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 64de39acc72176..9f6e2639508b2a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4926,10 +4926,12 @@ class Sema final {
SourceLocation EqualLoc);
void ActOnPureSpecifier(Decl *D, SourceLocation PureSpecLoc);
- void SetDeclDeleted(Decl *dcl, SourceLocation DelLoc);
+ void SetDeclDeleted(Decl *dcl, SourceLocation DelLoc,
+ StringLiteral *Message = nullptr);
void SetDeclDefaulted(Decl *dcl, SourceLocation DefaultLoc);
- void SetFunctionBodyKind(Decl *D, SourceLocation Loc, FnBodyKind BodyKind);
+ void SetFunctionBodyKind(Decl *D, SourceLocation Loc, FnBodyKind BodyKind,
+ StringLiteral *DeletedMessage = nullptr);
void ActOnStartTrailingRequiresClause(Scope *S, Declarator &D);
ExprResult ActOnFinishTrailingRequiresClause(ExprResult ConstraintExpr);
ExprResult ActOnRequiresClause(ExprResult ConstraintExpr);
@@ -8262,6 +8264,11 @@ class Sema final {
bool IsFunctionConversion(QualType FromType, QualType ToType,
QualType &ResultTy);
bool DiagnoseMultipleUserDefinedConversion(Expr *From, QualType ToType);
+ void DiagnoseUseOfDeletedFunction(SourceLocation Loc, SourceRange Range,
+ DeclarationName Name,
+ OverloadCandidateSet &CandidateSet,
+ FunctionDecl *Fn, MultiExprArg Args,
+ bool IsMember = false);
ExprResult InitializeExplicitObjectArgument(Sema &S, Expr *Obj,
FunctionDecl *Fun);
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 95900afdd2c5d8..b5e61f0eceb6e4 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3043,8 +3043,8 @@ FunctionDecl::FunctionDecl(Kind DK, ASTContext &C, DeclContext *DC,
Expr *TrailingRequiresClause)
: DeclaratorDecl(DK, DC, NameInfo.getLoc(), NameInfo.getName(), T, TInfo,
StartLoc),
- DeclContext(DK), redeclarable_base(C), Body(), ODRHash(0),
- EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) {
+ DeclContext(DK), redeclarable_base(C), Body(), DeletedMessage(nullptr),
+ ODRHash(0), EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) {
assert(T.isNull() || T->isFunctionType());
FunctionDeclBits.SClass = S;
FunctionDeclBits.IsInline = isInlineSpecified;
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index b701581b2474a9..d04cdd5a10e033 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -873,9 +873,14 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
if (D->isPureVirtual())
Out << " = 0";
- else if (D->isDeletedAsWritten())
+ else if (D->isDeletedAsWritten()) {
Out << " = delete";
- else if (D->isExplicitlyDefaulted())
+ if (const auto *M = D->getDeletedMessage()) {
+ Out << "(";
+ M->outputString(Out);
+ Out << ")";
+ }
+ } else if (D->isExplicitlyDefaulted())
Out << " = default";
else if (D->doesThisDeclarationHaveABody()) {
if (!Policy.TerseOutput) {
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index b683eb1edd8f13..4a2369330a35e7 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1936,6 +1936,9 @@ void TextNodeDumper::VisitFunctionDecl(const FunctionDecl *D) {
if (D->isTrivial())
OS << " trivial";
+ if (const auto *M = D->getDeletedMessage())
+ AddChild("delete message", [=] { Visit(M); });
+
if (D->isIneligibleOrNotSelected())
OS << (isa<CXXDestructorDecl>(D) ? " not_selected" : " ineligible");
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index d790060c17c049..9e2f4ce562a4ea 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -20,6 +20,28 @@
using namespace clang;
+/// Parse the optional ("message") part of a deleted-function-body.
+StringLiteral *Parser::ParseCXXDeletedFunctionMessage() {
+ if (!Tok.is(tok::l_paren))
+ return nullptr;
+ StringLiteral *Message = nullptr;
+ BalancedDelimiterTracker BT{*this, tok::l_paren};
+ BT.consumeOpen();
+
+ if (isTokenStringLiteral()) {
+ ExprResult Res = ParseUnevaluatedStringLiteralExpression();
+ if (Res.isUsable())
+ Message = Res.getAs<StringLiteral>();
+ } else {
+ Diag(Tok.getLocation(), diag::err_expected_string_literal)
+ << /*Source='in'*/ 0 << "'delete'";
+ SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch);
+ }
+
+ BT.consumeClose();
+ return Message;
+}
+
/// ParseCXXInlineMethodDef - We parsed and verified that the specified
/// Declarator is a well formed C++ inline method definition. Now lex its body
/// and store its tokens for parsing after the C++ class is complete.
@@ -70,7 +92,8 @@ NamedDecl *Parser::ParseCXXInlineMethodDef(
? diag::warn_cxx98_compat_defaulted_deleted_function
: diag::ext_defaulted_deleted_function)
<< 1 /* deleted */;
- Actions.SetDeclDeleted(FnD, KWLoc);
+ StringLiteral *Message = ParseCXXDeletedFunctionMessage();
+ Actions.SetDeclDeleted(FnD, KWLoc, Message);
Delete = true;
if (auto *DeclAsFunction = dyn_cast<FunctionDecl>(FnD)) {
DeclAsFunction->setRangeEnd(KWEndLoc);
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index cc0e41ed221c4f..d6f2b9f448cd52 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -1404,6 +1404,7 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
// Parse function body eagerly if it is either '= delete;' or '= default;' as
// ActOnStartOfFunctionDef needs to know whether the function is deleted.
+ StringLiteral *DeletedMessage = nullptr;
Sema::FnBodyKind BodyKind = Sema::FnBodyKind::Other;
SourceLocation KWLoc;
if (TryConsumeToken(tok::equal)) {
@@ -1415,6 +1416,7 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
: diag::ext_defaulted_deleted_function)
<< 1 /* deleted */;
BodyKind = Sema::FnBodyKind::Delete;
+ DeletedMessage = ParseCXXDeletedFunctionMessage();
} else if (TryConsumeToken(tok::kw_default, KWLoc)) {
Diag(KWLoc, getLangOpts().CPlusPlus11
? diag::warn_cxx98_compat_defaulted_deleted_function
@@ -1473,7 +1475,7 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
D.getMutableDeclSpec().abort();
if (BodyKind != Sema::FnBodyKind::Other) {
- Actions.SetFunctionBodyKind(Res, KWLoc, BodyKind);
+ Actions.SetFunctionBodyKind(Res, KWLoc, BodyKind, DeletedMessage);
Stmt *GeneratedBody = Res ? Res->getBody() : nullptr;
Actions.ActOnFinishFunctionBody(Res, GeneratedBody, false);
return Res;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index e258a4f7c89415..50eff9b02cbef8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18108,7 +18108,8 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
return ND;
}
-void Sema::SetDeclDeleted(Decl *Dcl, SourceLocation DelLoc) {
+void Sema::SetDeclDeleted(Decl *Dcl, SourceLocation DelLoc,
+ StringLiteral *Message) {
AdjustDeclIfTemplate(Dcl);
FunctionDecl *Fn = dyn_cast_or_null<FunctionDecl>(Dcl);
@@ -18157,7 +18158,7 @@ void Sema::SetDeclDeleted(Decl *Dcl, SourceLocation DelLoc) {
// C++11 [dcl.fct.def.delete]p4:
// A deleted function is implicitly inline.
Fn->setImplicitlyInline();
- Fn->setDeletedAsWritten();
+ Fn->setDeletedWithMessage(Message);
}
void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation DefaultLoc) {
@@ -18270,11 +18271,11 @@ void Sema::DiagnoseReturnInConstructorExceptionHandler(CXXTryStmt *TryBlock) {
}
}
-void Sema::SetFunctionBodyKind(Decl *D, SourceLocation Loc,
- FnBodyKind BodyKind) {
+void Sema::SetFunctionBodyKind(Decl *D, SourceLocation Loc, FnBodyKind BodyKind,
+ StringLiteral *DeletedMessage) {
switch (BodyKind) {
case FnBodyKind::Delete:
- SetDeclDeleted(D, Loc);
+ SetDeclDeleted(D, Loc, DeletedMessage);
break;
case FnBodyKind::Default:
SetDeclDefaulted(D, Loc);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8725b09f8546cf..76fbce8a95da85 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -271,8 +271,11 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,
Diag(Loc, diag::err_deleted_inherited_ctor_use)
<< Ctor->getParent()
<< Ctor->getInheritedConstructor().getConstructor()->getParent();
- else
- Diag(Loc, diag::err_deleted_function_use);
+ else {
+ StringLiteral *Msg = FD->getDeletedMessage();
+ Diag(Loc, diag::err_deleted_function_use)
+ << !!Msg << (Msg ? Msg->getString() : StringRef());
+ }
NoteDeletedFunction(FD);
return true;
}
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c34a40fa7c81ac..e5b8d1fbb81554 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2663,13 +2663,9 @@ static bool resolveAllocationOverload(
return true;
case OR_Deleted: {
- if (Diagnose) {
- Candidates.NoteCandidates(
- PartialDiagnosticAt(R.getNameLoc(),
- S.PDiag(diag::err_ovl_deleted_call)
- << R.getLookupName() << Range),
- S, OCD_AllCandidates, Args);
- }
+ if (Diagnose)
+ S.DiagnoseUseOfDeletedFunction(R.getNameLoc(), Range, R.getLookupName(),
+ Candidates, Best->Function, Args);
return true;
}
}
@@ -3323,7 +3319,9 @@ bool Sema::FindDeallocationFunction(SourceLocation StartLoc, CXXRecordDecl *RD,
// FIXME: DiagnoseUseOfDecl?
if (Operator->isDeleted()) {
if (Diagnose) {
- Diag(StartLoc, diag::err_deleted_function_use);
+ StringLiteral *Msg = Operator->getDeletedMessage();
+ Diag(StartLoc, diag::err_deleted_function_use)
+ << !!Msg << (Msg ? Msg->getString() : StringRef());
NoteDeletedFunction(Operator);
}
return true;
@@ -3927,14 +3925,11 @@ static bool resolveBuiltinNewDeleteOverload(Sema &S, CallExpr *TheCall,
S, OCD_AmbiguousCandidates, Args);
return true;
- case OR_Deleted: {
- Candidates.NoteCandidates(
- PartialDiagnosticAt(R.getNameLoc(), S.PDiag(diag::err_ovl_deleted_call)
- << R.getLookupName() << Range),
- S, OCD_AllCandidates, Args);
+ case OR_Deleted:
+ S.DiagnoseUseOfDeletedFunction(R.getNameLoc(), Range, R.getLookupName(),
+ Candidates, Best->Function, Args);
return true;
}
- }
llvm_unreachable("Unreachable, bad result from BestViableFunction");
}
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 4e03c3124e39ab..e4350df47c3570 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -14138,15 +14138,13 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
break;
case OR_Deleted: {
- CandidateSet->NoteCandidates(
- PartialDiagnosticAt(Fn->getBeginLoc(),
- SemaRef.PDiag(diag::err_ovl_deleted_call)
- << ULE->getName() << Fn->getSourceRange()),
- SemaRef, OCD_AllCandidates, Args);
+ FunctionDecl *FDecl = (*Best)->Function;
+ SemaRef.DiagnoseUseOfDeletedFunction(Fn->getBeginLoc(),
+ Fn->getSourceRange(), ULE->getName(),
+ *CandidateSet, FDecl, Args);
// We emitted an error for the unavailable/deleted function call but keep
// the call in the AST.
- FunctionDecl *FDecl = (*Best)->Function;
ExprResult Res =
SemaRef.FixOverloadedFunctionReference(Fn, (*Best)->FoundDecl, FDecl);
if (Res.isInvalid())
@@ -15588,11 +15586,9 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE,
*this, OCD_AmbiguousCandidates, Args);
break;
case OR_Deleted:
- CandidateSet.NoteCandidates(
- PartialDiagnosticAt(UnresExpr->getMemberLoc(),
- PDiag(diag::err_ovl_deleted_member_call)
- << DeclName << MemExprE->getSourceRange()),
- *this, OCD_AllCandidates, Args);
+ DiagnoseUseOfDeletedFunction(
+ UnresExpr->getMemberLoc(), MemExprE->getSourceRange(), DeclName,
+ CandidateSet, Best->Function, Args, /*IsMember=*/true);
break;
}
// Overload resolution fails, try to recover.
@@ -16483,3 +16479,17 @@ bool clang::shouldEnforceArgLimit(bool PartialOverloading,
return false;
return true;
}
+
+void Sema::DiagnoseUseOfDeletedFunction(SourceLocation Loc, SourceRange Range,
+ DeclarationName Name,
+ OverloadCandidateSet &CandidateSet,
+ FunctionDecl *Fn, MultiExprArg Args,
+ bool IsMember) {
+ StringLiteral *Msg = Fn->getDeletedMessage();
+ CandidateSet.NoteCandidates(
+ PartialDiagnosticAt(Loc, PDiag(diag::err_ovl_deleted_call)
+ << IsMember << Name << !!Msg
+ << (Msg ? Msg->getString() : StringRef())
+ << Range),
+ *this, OCD_AllCandidates, Args);
+}
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index dc972018e7b281..dfc6edc003e641 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2429,7 +2429,7 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(
return nullptr;
}
if (D->isDeleted())
- SemaRef.SetDeclDeleted(Function, D->getLocation());
+ SemaRef.SetDeclDeleted(Function, D->getLocation(), D->getDeletedMessage());
NamedDecl *PrincipalDecl =
(TemplateParams ? cast<NamedDecl>(FunctionTemplate) : Function);
@@ -2805,7 +2805,8 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
return nullptr;
}
if (D->isDeletedAsWritten())
- SemaRef.SetDeclDeleted(Method, Method->getLocation());
+ SemaRef.SetDeclDeleted(Method, Method->getLocation(),
+ D->getDeletedMessage());
// If this is an explicit specialization, mark the implicitly-instantiated
// template specialization as being an explicit specialization too.
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index a22f760408c634..37bc61b9a63a34 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1122,6 +1122,11 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
}
}
+ // FIXME: See ASTWriterDecl::VisitFunctionDecl.
+ if (FD->isDeletedAsWritten())
+ FD->setDeletedWithMessage(
+ cast_if_present<StringLiteral>(Record.readStmt()));
+
if (Existing)
mergeRedeclarable(FD, Existing, Redecl);
else if (auto Kind = FD->getTemplatedKind();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 86f64bf2a24250..af7dbe916419eb 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -761,6 +761,13 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) {
}
}
+ // FIXME: Hack: We're out of bits in FunctionDeclBits, so always
+ // add this even though it's 0 in the vast majority of cases. We
+ // might really want to consider storing this in the DefaultedFunctionInfo
+ // instead.
+ if (D->isDeletedAsWritten())
+ Record.AddStmt(D->getDeletedMessage());
+
Record.push_back(D->param_size());
for (auto *P : D->parameters())
Record.AddDeclRef(P);
diff --git a/clang/test/AST/ast-dump-cxx2c-delete-with-message.cpp b/clang/test/AST/ast-dump-cxx2c-delete-with-message.cpp
new file mode 100644
index 00000000000000..ea16b97da23e40
--- /dev/null
+++ b/clang/test/AST/ast-dump-cxx2c-delete-with-message.cpp
@@ -0,0 +1,23 @@
+// Without serialization:
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+//
+// With serialization:
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -include-pch %t -ast-dump...
[truncated]
|
@llvm/pr-subscribers-clang Author: None (Sirraide) ChangesThis implements support for the The major parts are all done, from what I can tell, except for one thing: we need to figure out where to store the message. We’re out of bits in There are some fairly straight-forward things that are still missing; I’m putting these here so I don’t end up forgetting about any of them (I’ll work on these while we figure out where to put the message):
Patch is 24.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86526.diff 20 Files Affected:
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index a5879591f4c659..9c445eccdf8992 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2013,6 +2013,12 @@ class FunctionDecl : public DeclaratorDecl,
DefaultedFunctionInfo *DefaultedInfo;
};
+ /// Message that indicates why this function was deleted.
+ ///
+ /// FIXME: Figure out where to actually put this; maybe in the
+ /// 'DefaultedInfo' above?
+ StringLiteral *DeletedMessage;
+
unsigned ODRHash;
/// End part of this FunctionDecl's source range.
@@ -2483,6 +2489,10 @@ class FunctionDecl : public DeclaratorDecl,
}
void setDeletedAsWritten(bool D = true) { FunctionDeclBits.IsDeleted = D; }
+ void setDeletedWithMessage(StringLiteral *Message) {
+ FunctionDeclBits.IsDeleted = true;
+ DeletedMessage = Message;
+ }
/// Determines whether this function is "main", which is the
/// entry point into an executable program.
@@ -2638,6 +2648,9 @@ class FunctionDecl : public DeclaratorDecl,
AC.push_back(TRC);
}
+ /// Get the message that indicates why this function was deleted.
+ StringLiteral *getDeletedMessage() const { return DeletedMessage; }
+
void setPreviousDeclaration(FunctionDecl * PrevDecl);
FunctionDecl *getCanonicalDecl() override;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d7ab1635cf12bc..37197b60ddfbb8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4661,11 +4661,10 @@ def err_ovl_no_viable_member_function_in_call : Error<
"no matching member function for call to %0">;
def err_ovl_ambiguous_call : Error<
"call to %0 is ambiguous">;
-def err_ovl_deleted_call : Error<"call to deleted function %0">;
+def err_ovl_deleted_call : Error<"call to deleted"
+ "%select{| member}0 function %1%select{|: %3}2">;
def err_ovl_ambiguous_member_call : Error<
"call to member function %0 is ambiguous">;
-def err_ovl_deleted_member_call : Error<
- "call to deleted member function %0">;
def note_ovl_too_many_candidates : Note<
"remaining %0 candidate%s0 omitted; "
"pass -fshow-overloads=all to show them">;
@@ -8857,7 +8856,7 @@ def err_nontemporal_builtin_must_be_pointer_intfltptr_or_vector : Error<
"address argument to nontemporal builtin must be a pointer to integer, float, "
"pointer, or a vector of such types (%0 invalid)">;
-def err_deleted_function_use : Error<"attempt to use a deleted function">;
+def err_deleted_function_use : Error<"attempt to use a deleted function%select{|: %1}0">;
def err_deleted_inherited_ctor_use : Error<
"constructor inherited by %0 from base class %1 is implicitly deleted">;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 14df75180ef321..559e6416b7dfb3 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1600,6 +1600,7 @@ class Parser : public CodeCompletionHandler {
const ParsedTemplateInfo &TemplateInfo,
const VirtSpecifiers &VS,
SourceLocation PureSpecLoc);
+ StringLiteral *ParseCXXDeletedFunctionMessage();
void ParseCXXNonStaticMemberInitializer(Decl *VarD);
void ParseLexedAttributes(ParsingClass &Class);
void ParseLexedAttributeList(LateParsedAttrList &LAs, Decl *D,
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 64de39acc72176..9f6e2639508b2a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4926,10 +4926,12 @@ class Sema final {
SourceLocation EqualLoc);
void ActOnPureSpecifier(Decl *D, SourceLocation PureSpecLoc);
- void SetDeclDeleted(Decl *dcl, SourceLocation DelLoc);
+ void SetDeclDeleted(Decl *dcl, SourceLocation DelLoc,
+ StringLiteral *Message = nullptr);
void SetDeclDefaulted(Decl *dcl, SourceLocation DefaultLoc);
- void SetFunctionBodyKind(Decl *D, SourceLocation Loc, FnBodyKind BodyKind);
+ void SetFunctionBodyKind(Decl *D, SourceLocation Loc, FnBodyKind BodyKind,
+ StringLiteral *DeletedMessage = nullptr);
void ActOnStartTrailingRequiresClause(Scope *S, Declarator &D);
ExprResult ActOnFinishTrailingRequiresClause(ExprResult ConstraintExpr);
ExprResult ActOnRequiresClause(ExprResult ConstraintExpr);
@@ -8262,6 +8264,11 @@ class Sema final {
bool IsFunctionConversion(QualType FromType, QualType ToType,
QualType &ResultTy);
bool DiagnoseMultipleUserDefinedConversion(Expr *From, QualType ToType);
+ void DiagnoseUseOfDeletedFunction(SourceLocation Loc, SourceRange Range,
+ DeclarationName Name,
+ OverloadCandidateSet &CandidateSet,
+ FunctionDecl *Fn, MultiExprArg Args,
+ bool IsMember = false);
ExprResult InitializeExplicitObjectArgument(Sema &S, Expr *Obj,
FunctionDecl *Fun);
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 95900afdd2c5d8..b5e61f0eceb6e4 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3043,8 +3043,8 @@ FunctionDecl::FunctionDecl(Kind DK, ASTContext &C, DeclContext *DC,
Expr *TrailingRequiresClause)
: DeclaratorDecl(DK, DC, NameInfo.getLoc(), NameInfo.getName(), T, TInfo,
StartLoc),
- DeclContext(DK), redeclarable_base(C), Body(), ODRHash(0),
- EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) {
+ DeclContext(DK), redeclarable_base(C), Body(), DeletedMessage(nullptr),
+ ODRHash(0), EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) {
assert(T.isNull() || T->isFunctionType());
FunctionDeclBits.SClass = S;
FunctionDeclBits.IsInline = isInlineSpecified;
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index b701581b2474a9..d04cdd5a10e033 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -873,9 +873,14 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
if (D->isPureVirtual())
Out << " = 0";
- else if (D->isDeletedAsWritten())
+ else if (D->isDeletedAsWritten()) {
Out << " = delete";
- else if (D->isExplicitlyDefaulted())
+ if (const auto *M = D->getDeletedMessage()) {
+ Out << "(";
+ M->outputString(Out);
+ Out << ")";
+ }
+ } else if (D->isExplicitlyDefaulted())
Out << " = default";
else if (D->doesThisDeclarationHaveABody()) {
if (!Policy.TerseOutput) {
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index b683eb1edd8f13..4a2369330a35e7 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1936,6 +1936,9 @@ void TextNodeDumper::VisitFunctionDecl(const FunctionDecl *D) {
if (D->isTrivial())
OS << " trivial";
+ if (const auto *M = D->getDeletedMessage())
+ AddChild("delete message", [=] { Visit(M); });
+
if (D->isIneligibleOrNotSelected())
OS << (isa<CXXDestructorDecl>(D) ? " not_selected" : " ineligible");
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index d790060c17c049..9e2f4ce562a4ea 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -20,6 +20,28 @@
using namespace clang;
+/// Parse the optional ("message") part of a deleted-function-body.
+StringLiteral *Parser::ParseCXXDeletedFunctionMessage() {
+ if (!Tok.is(tok::l_paren))
+ return nullptr;
+ StringLiteral *Message = nullptr;
+ BalancedDelimiterTracker BT{*this, tok::l_paren};
+ BT.consumeOpen();
+
+ if (isTokenStringLiteral()) {
+ ExprResult Res = ParseUnevaluatedStringLiteralExpression();
+ if (Res.isUsable())
+ Message = Res.getAs<StringLiteral>();
+ } else {
+ Diag(Tok.getLocation(), diag::err_expected_string_literal)
+ << /*Source='in'*/ 0 << "'delete'";
+ SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch);
+ }
+
+ BT.consumeClose();
+ return Message;
+}
+
/// ParseCXXInlineMethodDef - We parsed and verified that the specified
/// Declarator is a well formed C++ inline method definition. Now lex its body
/// and store its tokens for parsing after the C++ class is complete.
@@ -70,7 +92,8 @@ NamedDecl *Parser::ParseCXXInlineMethodDef(
? diag::warn_cxx98_compat_defaulted_deleted_function
: diag::ext_defaulted_deleted_function)
<< 1 /* deleted */;
- Actions.SetDeclDeleted(FnD, KWLoc);
+ StringLiteral *Message = ParseCXXDeletedFunctionMessage();
+ Actions.SetDeclDeleted(FnD, KWLoc, Message);
Delete = true;
if (auto *DeclAsFunction = dyn_cast<FunctionDecl>(FnD)) {
DeclAsFunction->setRangeEnd(KWEndLoc);
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index cc0e41ed221c4f..d6f2b9f448cd52 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -1404,6 +1404,7 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
// Parse function body eagerly if it is either '= delete;' or '= default;' as
// ActOnStartOfFunctionDef needs to know whether the function is deleted.
+ StringLiteral *DeletedMessage = nullptr;
Sema::FnBodyKind BodyKind = Sema::FnBodyKind::Other;
SourceLocation KWLoc;
if (TryConsumeToken(tok::equal)) {
@@ -1415,6 +1416,7 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
: diag::ext_defaulted_deleted_function)
<< 1 /* deleted */;
BodyKind = Sema::FnBodyKind::Delete;
+ DeletedMessage = ParseCXXDeletedFunctionMessage();
} else if (TryConsumeToken(tok::kw_default, KWLoc)) {
Diag(KWLoc, getLangOpts().CPlusPlus11
? diag::warn_cxx98_compat_defaulted_deleted_function
@@ -1473,7 +1475,7 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
D.getMutableDeclSpec().abort();
if (BodyKind != Sema::FnBodyKind::Other) {
- Actions.SetFunctionBodyKind(Res, KWLoc, BodyKind);
+ Actions.SetFunctionBodyKind(Res, KWLoc, BodyKind, DeletedMessage);
Stmt *GeneratedBody = Res ? Res->getBody() : nullptr;
Actions.ActOnFinishFunctionBody(Res, GeneratedBody, false);
return Res;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index e258a4f7c89415..50eff9b02cbef8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18108,7 +18108,8 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
return ND;
}
-void Sema::SetDeclDeleted(Decl *Dcl, SourceLocation DelLoc) {
+void Sema::SetDeclDeleted(Decl *Dcl, SourceLocation DelLoc,
+ StringLiteral *Message) {
AdjustDeclIfTemplate(Dcl);
FunctionDecl *Fn = dyn_cast_or_null<FunctionDecl>(Dcl);
@@ -18157,7 +18158,7 @@ void Sema::SetDeclDeleted(Decl *Dcl, SourceLocation DelLoc) {
// C++11 [dcl.fct.def.delete]p4:
// A deleted function is implicitly inline.
Fn->setImplicitlyInline();
- Fn->setDeletedAsWritten();
+ Fn->setDeletedWithMessage(Message);
}
void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation DefaultLoc) {
@@ -18270,11 +18271,11 @@ void Sema::DiagnoseReturnInConstructorExceptionHandler(CXXTryStmt *TryBlock) {
}
}
-void Sema::SetFunctionBodyKind(Decl *D, SourceLocation Loc,
- FnBodyKind BodyKind) {
+void Sema::SetFunctionBodyKind(Decl *D, SourceLocation Loc, FnBodyKind BodyKind,
+ StringLiteral *DeletedMessage) {
switch (BodyKind) {
case FnBodyKind::Delete:
- SetDeclDeleted(D, Loc);
+ SetDeclDeleted(D, Loc, DeletedMessage);
break;
case FnBodyKind::Default:
SetDeclDefaulted(D, Loc);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8725b09f8546cf..76fbce8a95da85 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -271,8 +271,11 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,
Diag(Loc, diag::err_deleted_inherited_ctor_use)
<< Ctor->getParent()
<< Ctor->getInheritedConstructor().getConstructor()->getParent();
- else
- Diag(Loc, diag::err_deleted_function_use);
+ else {
+ StringLiteral *Msg = FD->getDeletedMessage();
+ Diag(Loc, diag::err_deleted_function_use)
+ << !!Msg << (Msg ? Msg->getString() : StringRef());
+ }
NoteDeletedFunction(FD);
return true;
}
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c34a40fa7c81ac..e5b8d1fbb81554 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2663,13 +2663,9 @@ static bool resolveAllocationOverload(
return true;
case OR_Deleted: {
- if (Diagnose) {
- Candidates.NoteCandidates(
- PartialDiagnosticAt(R.getNameLoc(),
- S.PDiag(diag::err_ovl_deleted_call)
- << R.getLookupName() << Range),
- S, OCD_AllCandidates, Args);
- }
+ if (Diagnose)
+ S.DiagnoseUseOfDeletedFunction(R.getNameLoc(), Range, R.getLookupName(),
+ Candidates, Best->Function, Args);
return true;
}
}
@@ -3323,7 +3319,9 @@ bool Sema::FindDeallocationFunction(SourceLocation StartLoc, CXXRecordDecl *RD,
// FIXME: DiagnoseUseOfDecl?
if (Operator->isDeleted()) {
if (Diagnose) {
- Diag(StartLoc, diag::err_deleted_function_use);
+ StringLiteral *Msg = Operator->getDeletedMessage();
+ Diag(StartLoc, diag::err_deleted_function_use)
+ << !!Msg << (Msg ? Msg->getString() : StringRef());
NoteDeletedFunction(Operator);
}
return true;
@@ -3927,14 +3925,11 @@ static bool resolveBuiltinNewDeleteOverload(Sema &S, CallExpr *TheCall,
S, OCD_AmbiguousCandidates, Args);
return true;
- case OR_Deleted: {
- Candidates.NoteCandidates(
- PartialDiagnosticAt(R.getNameLoc(), S.PDiag(diag::err_ovl_deleted_call)
- << R.getLookupName() << Range),
- S, OCD_AllCandidates, Args);
+ case OR_Deleted:
+ S.DiagnoseUseOfDeletedFunction(R.getNameLoc(), Range, R.getLookupName(),
+ Candidates, Best->Function, Args);
return true;
}
- }
llvm_unreachable("Unreachable, bad result from BestViableFunction");
}
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 4e03c3124e39ab..e4350df47c3570 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -14138,15 +14138,13 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
break;
case OR_Deleted: {
- CandidateSet->NoteCandidates(
- PartialDiagnosticAt(Fn->getBeginLoc(),
- SemaRef.PDiag(diag::err_ovl_deleted_call)
- << ULE->getName() << Fn->getSourceRange()),
- SemaRef, OCD_AllCandidates, Args);
+ FunctionDecl *FDecl = (*Best)->Function;
+ SemaRef.DiagnoseUseOfDeletedFunction(Fn->getBeginLoc(),
+ Fn->getSourceRange(), ULE->getName(),
+ *CandidateSet, FDecl, Args);
// We emitted an error for the unavailable/deleted function call but keep
// the call in the AST.
- FunctionDecl *FDecl = (*Best)->Function;
ExprResult Res =
SemaRef.FixOverloadedFunctionReference(Fn, (*Best)->FoundDecl, FDecl);
if (Res.isInvalid())
@@ -15588,11 +15586,9 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE,
*this, OCD_AmbiguousCandidates, Args);
break;
case OR_Deleted:
- CandidateSet.NoteCandidates(
- PartialDiagnosticAt(UnresExpr->getMemberLoc(),
- PDiag(diag::err_ovl_deleted_member_call)
- << DeclName << MemExprE->getSourceRange()),
- *this, OCD_AllCandidates, Args);
+ DiagnoseUseOfDeletedFunction(
+ UnresExpr->getMemberLoc(), MemExprE->getSourceRange(), DeclName,
+ CandidateSet, Best->Function, Args, /*IsMember=*/true);
break;
}
// Overload resolution fails, try to recover.
@@ -16483,3 +16479,17 @@ bool clang::shouldEnforceArgLimit(bool PartialOverloading,
return false;
return true;
}
+
+void Sema::DiagnoseUseOfDeletedFunction(SourceLocation Loc, SourceRange Range,
+ DeclarationName Name,
+ OverloadCandidateSet &CandidateSet,
+ FunctionDecl *Fn, MultiExprArg Args,
+ bool IsMember) {
+ StringLiteral *Msg = Fn->getDeletedMessage();
+ CandidateSet.NoteCandidates(
+ PartialDiagnosticAt(Loc, PDiag(diag::err_ovl_deleted_call)
+ << IsMember << Name << !!Msg
+ << (Msg ? Msg->getString() : StringRef())
+ << Range),
+ *this, OCD_AllCandidates, Args);
+}
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index dc972018e7b281..dfc6edc003e641 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2429,7 +2429,7 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(
return nullptr;
}
if (D->isDeleted())
- SemaRef.SetDeclDeleted(Function, D->getLocation());
+ SemaRef.SetDeclDeleted(Function, D->getLocation(), D->getDeletedMessage());
NamedDecl *PrincipalDecl =
(TemplateParams ? cast<NamedDecl>(FunctionTemplate) : Function);
@@ -2805,7 +2805,8 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
return nullptr;
}
if (D->isDeletedAsWritten())
- SemaRef.SetDeclDeleted(Method, Method->getLocation());
+ SemaRef.SetDeclDeleted(Method, Method->getLocation(),
+ D->getDeletedMessage());
// If this is an explicit specialization, mark the implicitly-instantiated
// template specialization as being an explicit specialization too.
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index a22f760408c634..37bc61b9a63a34 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1122,6 +1122,11 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
}
}
+ // FIXME: See ASTWriterDecl::VisitFunctionDecl.
+ if (FD->isDeletedAsWritten())
+ FD->setDeletedWithMessage(
+ cast_if_present<StringLiteral>(Record.readStmt()));
+
if (Existing)
mergeRedeclarable(FD, Existing, Redecl);
else if (auto Kind = FD->getTemplatedKind();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 86f64bf2a24250..af7dbe916419eb 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -761,6 +761,13 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) {
}
}
+ // FIXME: Hack: We're out of bits in FunctionDeclBits, so always
+ // add this even though it's 0 in the vast majority of cases. We
+ // might really want to consider storing this in the DefaultedFunctionInfo
+ // instead.
+ if (D->isDeletedAsWritten())
+ Record.AddStmt(D->getDeletedMessage());
+
Record.push_back(D->param_size());
for (auto *P : D->parameters())
Record.AddDeclRef(P);
diff --git a/clang/test/AST/ast-dump-cxx2c-delete-with-message.cpp b/clang/test/AST/ast-dump-cxx2c-delete-with-message.cpp
new file mode 100644
index 00000000000000..ea16b97da23e40
--- /dev/null
+++ b/clang/test/AST/ast-dump-cxx2c-delete-with-message.cpp
@@ -0,0 +1,23 @@
+// Without serialization:
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+//
+// With serialization:
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -include-pch %t -ast-dump...
[truncated]
|
= delete("should have a reason");
= delete("should have a reason");
✅ With the latest revision this PR passed the Python code formatter. |
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on that, I did a quick preliminary review
The last bullet point of your todo list should be done as a follow up PR. everything else can/should be done here. |
Yeah, that was my thinking too more or less; that’s also why I listed it as ‘not necessary’—and I’m also not that familiar w/ libclang or AST matchers. |
if (D->isDeletedAsWritten()) | ||
Record.AddStmt(D->getDeletedMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My instinct reaction to this is that we should better use AddString
and readString
pairs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to go with that, but I couldn’t find any examples of that being done for string literals (at least not in this file), so I wasn’t sure whether that’d be the right approach, but it should be fine since it’s just a string literal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't understand. Since the string literal is required to be unevaluated string
, I feel it may be good to record the string value directly by StringLiteral::getBytes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't understand. Since the string literal is required to be
unevaluated string
, I feel it may be good to record the string value directly byStringLiteral::getBytes
.
Hmm, from what I can tell, we don’t usually do that for string literals though. E.g. the string literal of a FileScopeASMDecl
is also serialised as an expression:
llvm-project/clang/lib/Serialization/ASTWriterDecl.cpp
Lines 1272 to 1274 in ff870ae
void ASTDeclWriter::VisitFileScopeAsmDecl(FileScopeAsmDecl *D) { | |
VisitDecl(D); | |
Record.AddStmt(D->getAsmString()); |
Also, one issue that I see w/ only storing the string data is that we’d lose e.g. the source location of the string literal.
Co-authored-by: Mariya Podchishchaeva <[email protected]>
Co-authored-by: Mariya Podchishchaeva <[email protected]>
So, the message is now stored in the |
Yeah, it just seems to me that, generally, any code that follows this pattern: if (condition) {
// ... (assuming no control flow out of this 'if' here of course)
llvm_unreachable("message");
} should really just be assert(!condition && "message"); My reasoning is simply this: With assertions enabled, we get a crash either way, and with assertions disabled, the But also, this isn’t really specific to this part of the codebase, and it’s also out of scope for this pr; we should probably come back to this at some point, though. |
Forgot to mention it explicitly, but I’ve added more tests and implemented this for all the other sema diagnostics that had anything to do w/ explicitly deleted functions (there are a few that say something along the lines of ‘function has been implicitly deleted’, but I didn’t really look at those since this is about explicitly deleting functions). |
I wanted to make you aware of this new core issue https://cplusplus.github.io/CWG/issues/2876.html (which i think we should have tests for). Thanks |
So, as of now, this using T = void ();
using U = int;
T a = delete ("hello");
U b = delete ("hello"), c, d = delete ("hello");
struct C {
T e = delete ("hello");
U f = delete ("hello");
}; gives
which seems reasonable to me—unless there’s something I’m missing here, but it’s ill-formed either way, so so long as we’re diagnosing it, we’re fine, from what I can tell at least. One thing I did just now is made sure we discard the |
I’ve also added a parser test for that as well just now. |
Thanks for the new tests. did you see this comment? #86526 (review) |
Thanks for reminding me; I did see that one but ended up forgetting about it because I was sick for pretty much all of last week; I’ll take a look at it later. |
Just added the deleted message to the ODR hash as well. Not too familiar w/ ODR checking though, so I wasn’t sure whether to include it only if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last batch of nitpicks, and then I think I'll be happy enough to approve this patch!
4848200
to
c12f90e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting up with my comments and working on this feature.
I'm pretty happy with the state of the PR.
Please give it a day or two in case @erichkeane @AaronBallman @shafik @Fznamznon have further comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 set of 'nits' a few places, else LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, this LGTM, thank you! I did spot some minor stuff, but feel free to fix it up when landing.
llvm#86526) This implements support for the `= delete("message")` syntax that was only just added to C++26 ([P2573R2](https://isocpp.org/files/papers/P2573R2.html#proposal-scope)).
void setDefaultedFunctionInfo(DefaultedFunctionInfo *Info); | ||
DefaultedFunctionInfo *getDefaultedFunctionInfo() const; | ||
void setDefaultedOrDeletedInfo(DefaultedOrDeletedFunctionInfo *Info); | ||
DefaultedOrDeletedFunctionInfo *getDefalutedOrDeletedInfo() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sirraide There's no reason we can't change this name to fix this typo right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That can be fixed as NFC commit. I missed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sirraide There's no reason we can't change this name to fix this typo right?
Ah, my bad, yeah, that should just be a NFC fix.
This implements support for the
= delete("message")
syntax that was only just added to C++26 (P2573R2).The major parts are all done, from what I can tell, except for one thing: we need to figure out where to store the message. We’re out of bits in
FunctionDeclBits
, and we can’t put it in the union that stores theDefaultedFunctionInfo
because a function may end up having to store both at the same time, so I’m thinking we could store it in theDefaultedFunctionInfo
and rename it toExtraFunctionInfo
or something like that.There are some fairly straight-forward things that are still missing; I’m putting these here so I don’t end up forgetting about any of them (I’ll work on these while we figure out where to put the message):
= delete
if we have that in a comment somewhere, etc.= delete
).Not ‘necessary’ but would be nice: AST Matchers & libclang support for getting the message.(should probably be a separate pr)