[Clang] Implement C++26’s P2893R3 ‘Variadic friends’ #101448
Conversation
Sirraide
added
clang:frontend
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang-modules Author: None (Sirraide) ChangesImplement P2893R3 ‘Variadic friends’ for C++26. The implementation proper is mostly done, from what I can tell, but I have a few questions, so I thought I’d be best to list them all here (some are for me to investigate, some I’d appreciate it if anyone would happen to know the answer to them):
Thanks also to @zyn0217, who was working on this before me; I’ve taken over working on this since they didn’t have time for it anymore. I’ve incorporated parts of their WIP patch, so I’ll credit them as a co-author whenever this gets merged. This closes #98587. Patch is 43.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/101448.diff 27 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3c2e0282d1c72..aa8bb903d3646 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -89,6 +89,8 @@ C++2c Feature Support
- Add ``__builtin_is_virtual_base_of`` intrinsic, which supports
`P2985R0 A type trait for detecting virtual base classes <https://wg21.link/p2985r0>`_
+- Implemented `P2893R3 Variadic Friends <https://wg21.link/P2893>`_
+
Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/DeclFriend.h b/clang/include/clang/AST/DeclFriend.h
index 9789282f351a5..19dd531e89ccf 100644
--- a/clang/include/clang/AST/DeclFriend.h
+++ b/clang/include/clang/AST/DeclFriend.h
@@ -70,6 +70,9 @@ class FriendDecl final
// Location of the 'friend' specifier.
SourceLocation FriendLoc;
+ // Location of the '...', if present.
+ SourceLocation EllipsisLoc;
+
/// True if this 'friend' declaration is unsupported. Eventually we
/// will support every possible friend declaration, but for now we
/// silently ignore some and set this flag to authorize all access.
@@ -82,10 +85,11 @@ class FriendDecl final
unsigned NumTPLists : 31;
FriendDecl(DeclContext *DC, SourceLocation L, FriendUnion Friend,
- SourceLocation FriendL,
+ SourceLocation FriendL, SourceLocation EllipsisLoc,
ArrayRef<TemplateParameterList *> FriendTypeTPLists)
: Decl(Decl::Friend, DC, L), Friend(Friend), FriendLoc(FriendL),
- UnsupportedFriend(false), NumTPLists(FriendTypeTPLists.size()) {
+ EllipsisLoc(EllipsisLoc), UnsupportedFriend(false),
+ NumTPLists(FriendTypeTPLists.size()) {
for (unsigned i = 0; i < NumTPLists; ++i)
getTrailingObjects<TemplateParameterList *>()[i] = FriendTypeTPLists[i];
}
@@ -110,7 +114,7 @@ class FriendDecl final
static FriendDecl *
Create(ASTContext &C, DeclContext *DC, SourceLocation L, FriendUnion Friend_,
- SourceLocation FriendL,
+ SourceLocation FriendL, SourceLocation EllipsisLoc = {},
ArrayRef<TemplateParameterList *> FriendTypeTPLists = std::nullopt);
static FriendDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID,
unsigned FriendTypeNumTPLists);
@@ -143,8 +147,24 @@ class FriendDecl final
return FriendLoc;
}
+ /// Retrieves the location of the '...', if present.
+ SourceLocation getEllipsisLoc() const { return EllipsisLoc; }
+
/// Retrieves the source range for the friend declaration.
SourceRange getSourceRange() const override LLVM_READONLY {
+ if (TypeSourceInfo *TInfo = getFriendType()) {
+ SourceLocation StartL =
+ (NumTPLists == 0) ? getFriendLoc()
+ : getTrailingObjects<TemplateParameterList *>()[0]
+ ->getTemplateLoc();
+ SourceLocation EndL =
+ isVariadic() ? getEllipsisLoc() : TInfo->getTypeLoc().getEndLoc();
+ return SourceRange(StartL, EndL);
+ }
+
+ if (isVariadic())
+ return SourceRange(getFriendLoc(), getEllipsisLoc());
+
if (NamedDecl *ND = getFriendDecl()) {
if (const auto *FD = dyn_cast<FunctionDecl>(ND))
return FD->getSourceRange();
@@ -158,15 +178,8 @@ class FriendDecl final
}
return SourceRange(getFriendLoc(), ND->getEndLoc());
}
- else if (TypeSourceInfo *TInfo = getFriendType()) {
- SourceLocation StartL =
- (NumTPLists == 0) ? getFriendLoc()
- : getTrailingObjects<TemplateParameterList *>()[0]
- ->getTemplateLoc();
- return SourceRange(StartL, TInfo->getTypeLoc().getEndLoc());
- }
- else
- return SourceRange(getFriendLoc(), getLocation());
+
+ return SourceRange(getFriendLoc(), getLocation());
}
/// Determines if this friend kind is unsupported.
@@ -177,11 +190,57 @@ class FriendDecl final
UnsupportedFriend = Unsupported;
}
+ bool isVariadic() const { return EllipsisLoc.isValid(); }
+
// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == Decl::Friend; }
};
+class FriendPackDecl final
+ : public Decl,
+ private llvm::TrailingObjects<FriendPackDecl, FriendDecl *> {
+ FriendDecl *InstantiatedFrom;
+
+ /// The number of friend-declarations created by this pack expansion.
+ unsigned NumExpansions;
+
+ FriendPackDecl(DeclContext *DC, FriendDecl *InstantiatedFrom,
+ ArrayRef<FriendDecl *> FriendDecls)
+ : Decl(FriendPack, DC,
+ InstantiatedFrom ? InstantiatedFrom->getLocation()
+ : SourceLocation()),
+ InstantiatedFrom(InstantiatedFrom), NumExpansions(FriendDecls.size()) {
+ std::uninitialized_copy(FriendDecls.begin(), FriendDecls.end(),
+ getTrailingObjects<FriendDecl *>());
+ }
+
+public:
+ friend class ASTDeclReader;
+ friend class ASTDeclWriter;
+ friend TrailingObjects;
+
+ FriendDecl *getInstantiatedFromFriendDecl() const { return InstantiatedFrom; }
+
+ ArrayRef<FriendDecl *> expansions() const {
+ return llvm::ArrayRef(getTrailingObjects<FriendDecl *>(), NumExpansions);
+ }
+
+ static FriendPackDecl *Create(ASTContext &C, DeclContext *DC,
+ FriendDecl *InstantiatedFrom,
+ ArrayRef<FriendDecl *> FriendDecls);
+
+ static FriendPackDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID,
+ unsigned NumExpansions);
+
+ SourceRange getSourceRange() const override LLVM_READONLY {
+ return InstantiatedFrom->getSourceRange();
+ }
+
+ static bool classof(const Decl *D) { return classofKind(D->getKind()); }
+ static bool classofKind(Kind K) { return K == FriendPack; }
+};
+
/// An iterator over the friend declarations of a class.
class CXXRecordDecl::friend_iterator {
friend class CXXRecordDecl;
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index dcf5dbf449f8b..e49fd3f58db57 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1606,6 +1606,8 @@ DEF_TRAVERSE_DECL(FriendDecl, {
}
})
+DEF_TRAVERSE_DECL(FriendPackDecl, {})
+
DEF_TRAVERSE_DECL(FriendTemplateDecl, {
if (D->getFriendType())
TRY_TO(TraverseTypeLoc(D->getFriendType()->getTypeLoc()));
diff --git a/clang/include/clang/Basic/DeclNodes.td b/clang/include/clang/Basic/DeclNodes.td
index 48396e85c5ada..8464cce4a9ddf 100644
--- a/clang/include/clang/Basic/DeclNodes.td
+++ b/clang/include/clang/Basic/DeclNodes.td
@@ -98,6 +98,7 @@ def FileScopeAsm : DeclNode<Decl>;
def TopLevelStmt : DeclNode<Decl>, DeclContext;
def AccessSpec : DeclNode<Decl>;
def Friend : DeclNode<Decl>;
+def FriendPack : DeclNode<Decl>;
def FriendTemplate : DeclNode<Decl>;
def StaticAssert : DeclNode<Decl>;
def Block : DeclNode<Decl, "blocks">, DeclContext;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 581434d33c5c9..8daa2386ef64f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1738,6 +1738,8 @@ def ext_friend_tag_redecl_outside_namespace : ExtWarn<
"enclosing namespace is a Microsoft extension; add a nested name specifier">,
InGroup<MicrosoftUnqualifiedFriend>;
def err_pure_friend : Error<"friend declaration cannot have a pure-specifier">;
+def err_friend_template_decl_multiple_specifiers: Error<
+ "a friend declaration that befriends a template must contain exactly one type-specifier">;
def err_invalid_base_in_interface : Error<
"interface type cannot inherit from "
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2ec6367eccea0..07c8c2cef9cc3 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3800,7 +3800,8 @@ class Sema final : public SemaBase {
const ParsedAttributesView &DeclAttrs,
MultiTemplateParamsArg TemplateParams,
bool IsExplicitInstantiation,
- RecordDecl *&AnonRecord);
+ RecordDecl *&AnonRecord,
+ SourceLocation FriendEllipsisLoc = {});
/// BuildAnonymousStructOrUnion - Handle the declaration of an
/// anonymous structure or union. Anonymous unions are a C++ feature
@@ -5538,7 +5539,8 @@ class Sema final : public SemaBase {
/// parameters present at all, require proper matching, i.e.
/// template <> template \<class T> friend class A<int>::B;
Decl *ActOnFriendTypeDecl(Scope *S, const DeclSpec &DS,
- MultiTemplateParamsArg TemplateParams);
+ MultiTemplateParamsArg TemplateParams,
+ SourceLocation FriendEllipsisLoc);
NamedDecl *ActOnFriendFunctionDecl(Scope *S, Declarator &D,
MultiTemplateParamsArg TemplateParams);
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index 5dd0ba33f8a9c..92e3fce78b682 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1377,6 +1377,9 @@ enum DeclCode {
/// A FriendDecl record.
DECL_FRIEND,
+ /// A FriendPackDecl record.
+ DECL_FRIEND_PACK,
+
/// A FriendTemplateDecl record.
DECL_FRIEND_TEMPLATE,
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 103235547f482..46d07069af6ba 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -536,6 +536,7 @@ namespace clang {
ExpectedDecl VisitFieldDecl(FieldDecl *D);
ExpectedDecl VisitIndirectFieldDecl(IndirectFieldDecl *D);
ExpectedDecl VisitFriendDecl(FriendDecl *D);
+ ExpectedDecl VisitFriendPackDecl(FriendPackDecl *D);
ExpectedDecl VisitObjCIvarDecl(ObjCIvarDecl *D);
ExpectedDecl VisitVarDecl(VarDecl *D);
ExpectedDecl VisitImplicitParamDecl(ImplicitParamDecl *D);
@@ -4424,11 +4425,14 @@ ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
auto FriendLocOrErr = import(D->getFriendLoc());
if (!FriendLocOrErr)
return FriendLocOrErr.takeError();
+ auto EllipsisLocOrErr = import(D->getEllipsisLoc());
+ if (!EllipsisLocOrErr)
+ return EllipsisLocOrErr.takeError();
FriendDecl *FrD;
if (GetImportedOrCreateDecl(FrD, D, Importer.getToContext(), DC,
- *LocationOrErr, ToFU,
- *FriendLocOrErr, ToTPLists))
+ *LocationOrErr, ToFU, *FriendLocOrErr,
+ *EllipsisLocOrErr, ToTPLists))
return FrD;
FrD->setAccess(D->getAccess());
@@ -4437,6 +4441,31 @@ ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
return FrD;
}
+ExpectedDecl ASTNodeImporter::VisitFriendPackDecl(FriendPackDecl *D) {
+ // Import the major distinguishing characteristics of a declaration.
+ DeclContext *DC, *LexicalDC;
+ if (Error Err = ImportDeclContext(D, DC, LexicalDC))
+ return std::move(Err);
+
+ auto ToInstantiatedFromFriendOrErr =
+ Importer.Import(D->getInstantiatedFromFriendDecl());
+ if (!ToInstantiatedFromFriendOrErr)
+ return ToInstantiatedFromFriendOrErr.takeError();
+ SmallVector<FriendDecl *, 4> Expansions(D->expansions().size());
+ if (Error Err = ImportArrayChecked(D->expansions(), Expansions.begin()))
+ return std::move(Err);
+
+ FriendPackDecl *ToFriendPack;
+ if (GetImportedOrCreateDecl(ToFriendPack, D, Importer.getToContext(), DC,
+ cast<FriendDecl>(*ToInstantiatedFromFriendOrErr),
+ Expansions))
+ return ToFriendPack;
+
+ addDeclToContexts(D, ToFriendPack);
+
+ return ToFriendPack;
+}
+
ExpectedDecl ASTNodeImporter::VisitObjCIvarDecl(ObjCIvarDecl *D) {
// Import the major distinguishing characteristics of an ivar.
DeclContext *DC, *LexicalDC;
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index a1f70546bde42..ee856e73c0d62 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -949,6 +949,7 @@ unsigned Decl::getIdentifierNamespaceForKind(Kind DeclKind) {
// Never have names.
case Friend:
+ case FriendPack:
case FriendTemplate:
case AccessSpec:
case LinkageSpec:
diff --git a/clang/lib/AST/DeclFriend.cpp b/clang/lib/AST/DeclFriend.cpp
index 04b9b93699f36..2662f3f634da6 100644
--- a/clang/lib/AST/DeclFriend.cpp
+++ b/clang/lib/AST/DeclFriend.cpp
@@ -31,11 +31,11 @@ FriendDecl *FriendDecl::getNextFriendSlowCase() {
NextFriend.get(getASTContext().getExternalSource()));
}
-FriendDecl *FriendDecl::Create(ASTContext &C, DeclContext *DC,
- SourceLocation L,
- FriendUnion Friend,
- SourceLocation FriendL,
- ArrayRef<TemplateParameterList *> FriendTypeTPLists) {
+FriendDecl *
+FriendDecl::Create(ASTContext &C, DeclContext *DC, SourceLocation L,
+ FriendUnion Friend, SourceLocation FriendL,
+ SourceLocation EllipsisLoc,
+ ArrayRef<TemplateParameterList *> FriendTypeTPLists) {
#ifndef NDEBUG
if (Friend.is<NamedDecl *>()) {
const auto *D = Friend.get<NamedDecl*>();
@@ -56,8 +56,8 @@ FriendDecl *FriendDecl::Create(ASTContext &C, DeclContext *DC,
std::size_t Extra =
FriendDecl::additionalSizeToAlloc<TemplateParameterList *>(
FriendTypeTPLists.size());
- auto *FD = new (C, DC, Extra) FriendDecl(DC, L, Friend, FriendL,
- FriendTypeTPLists);
+ auto *FD = new (C, DC, Extra)
+ FriendDecl(DC, L, Friend, FriendL, EllipsisLoc, FriendTypeTPLists);
cast<CXXRecordDecl>(DC)->pushFriendDecl(FD);
return FD;
}
@@ -69,6 +69,26 @@ FriendDecl *FriendDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID,
return new (C, ID, Extra) FriendDecl(EmptyShell(), FriendTypeNumTPLists);
}
+FriendPackDecl *FriendPackDecl::Create(ASTContext &C, DeclContext *DC,
+ FriendDecl *InstantiatedFrom,
+ ArrayRef<FriendDecl *> FriendDecls) {
+ size_t Extra = additionalSizeToAlloc<FriendDecl *>(FriendDecls.size());
+ return new (C, DC, Extra) FriendPackDecl(DC, InstantiatedFrom, FriendDecls);
+}
+
+FriendPackDecl *FriendPackDecl::CreateDeserialized(ASTContext &C,
+ GlobalDeclID ID,
+ unsigned NumExpansions) {
+ size_t Extra = additionalSizeToAlloc<FriendDecl *>(NumExpansions);
+ auto *Result =
+ new (C, ID, Extra) FriendPackDecl(nullptr, nullptr, std::nullopt);
+ Result->NumExpansions = NumExpansions;
+ auto *Trail = Result->getTrailingObjects<FriendDecl *>();
+ for (unsigned I = 0; I != NumExpansions; ++I)
+ new (Trail + I) FriendDecl *(nullptr);
+ return Result;
+}
+
FriendDecl *CXXRecordDecl::getFirstFriend() const {
ExternalASTSource *Source = getParentASTContext().getExternalSource();
Decl *First = data().FirstFriend.get(Source);
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 26773a69ab9ac..0b4faaf0b39ef 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -67,7 +67,7 @@ namespace {
void VisitEnumConstantDecl(EnumConstantDecl *D);
void VisitEmptyDecl(EmptyDecl *D);
void VisitFunctionDecl(FunctionDecl *D);
- void VisitFriendDecl(FriendDecl *D);
+ void VisitFriendDecl(FriendDecl *D, bool FirstInGroup = true);
void VisitFieldDecl(FieldDecl *D);
void VisitVarDecl(VarDecl *D);
void VisitLabelDecl(LabelDecl *D);
@@ -487,7 +487,26 @@ void DeclPrinter::VisitDeclContext(DeclContext *DC, bool Indent) {
}
this->Indent();
- Visit(*D);
+
+ // Group friend declarations if need be.
+ if (isa<FriendDecl>(*D)) {
+ auto *FD = cast<FriendDecl>(*D);
+ VisitFriendDecl(FD);
+ SourceLocation FriendLoc = FD->getFriendLoc();
+
+ // Use a separate iterator; 'D' is always one declaration 'behind' in
+ // this loop; the last friend printed here (or the first printed just
+ // now before this loop if there are no subsequent friends) will be
+ // skipped by the '++D' of the outer loop.
+ for (DeclContext::decl_iterator It; It = std::next(D), It != DEnd; ++D) {
+ auto NextFriend = dyn_cast<FriendDecl>(*It);
+ if (!NextFriend || NextFriend->getFriendLoc() != FriendLoc)
+ break;
+ VisitFriendDecl(NextFriend, false);
+ }
+ } else {
+ Visit(*D);
+ }
// FIXME: Need to be able to tell the DeclPrinter when
const char *Terminator = nullptr;
@@ -862,13 +881,21 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
}
}
-void DeclPrinter::VisitFriendDecl(FriendDecl *D) {
+void DeclPrinter::VisitFriendDecl(FriendDecl *D, bool FirstInGroup) {
if (TypeSourceInfo *TSI = D->getFriendType()) {
unsigned NumTPLists = D->getFriendTypeNumTemplateParameterLists();
for (unsigned i = 0; i < NumTPLists; ++i)
printTemplateParameters(D->getFriendTypeTemplateParameterList(i));
- Out << "friend ";
- Out << " " << TSI->getType().getAsString(Policy);
+
+ // Hack to print friend declarations declared as a group, e.g.
+ // 'friend int, long;', instead of printing them as two separate
+ // FriendDecls, which they are in the AST.
+ if (FirstInGroup)
+ Out << "friend ";
+ else
+ Out << ", ";
+
+ Out << TSI->getType().getAsString(Policy);
}
else if (FunctionDecl *FD =
dyn_cast<FunctionDecl>(D->getFriendDecl())) {
@@ -885,6 +912,9 @@ void DeclPrinter::VisitFriendDecl(FriendDecl *D) {
Out << "friend ";
VisitRedeclarableTemplateDecl(CTD);
}
+
+ if (D->isVariadic())
+ Out << "...";
}
void DeclPrinter::VisitFieldDecl(FieldDecl *D) {
diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp
index fbfe92318dc5e..65a02a6b66149 100644
--- a/clang/lib/AST/ODRHash.cpp
+++ b/clang/lib/AST/ODRHash.cpp
@@ -461,6 +461,7 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
} else {
AddDecl(D->getFriendDecl());
}
+ Hash.AddBoolean(D->isVariadic());
}
void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D) {
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 5ba9523504258..585d88e2e031e 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -2694,6 +2694,8 @@ void TextNodeDumper::VisitAccessSpecDecl(const AccessSpecDecl *D) {
void TextNodeDumper::VisitFriendDecl(const FriendDecl *D) {
if (TypeSourceInfo *T = D->getFriendType())
dumpType(T->getType());
+ if (D->isVariadic())
+ OS << " variadic";
}
void TextNodeDumper::VisitObjCIvarDecl(const ObjCIvarDecl *D) {
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 882dbad456379..4aa8e8f321aad 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -96,6 +96,7 @@ void CodeGenFunction::EmitDecl(const Decl &D) {
case Decl::FileScopeAsm:
case Decl::TopLevelStmt:
case Decl::Friend:
+ case Decl::FriendPack:
case Decl::FriendTemplate:
case Decl::Block:
case Decl::Captured:
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 17b9ca7cb9910..d17e213ea4d3f 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -754,6 +754,10 @@ static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
Builder.defineMacro("__cpp_multidimensional_subscript", "202211L");
Builder.defineMacro("__cpp_auto_cast", "202110L");
}
+ // C++26 features.
+ if (LangOpts.CPlusPlus26) {
+ Builder.defineMacro("__cpp_variadic_friend", "202403L");
+ }
// We provide those C++23 features as extensions in earlier language modes, so
// we also define their feature tes...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
If you do this now, you'll get a lot of non-trivial updates. Ideally you should wait for #97200 (@MitalAshok any blockers there?). |
zyn0217
left a comment
zyn0217
left a comment
There was a problem hiding this comment.
Thanks so much for continuing the effort to implement this! I haven't dived into the details yet, so here are some nits from me.
(I'd also prefer the pronouns he/him if you'd like. ;)
|
CI failure seems unrelated |
cor3ntin
left a comment
cor3ntin
left a comment
There was a problem hiding this comment.
Looks generally good from a cursory review.
I think we want to add tests for modules / in the test/PCH directory.
We probably want that as an extension indeed, no reason not to
We probably want a separate PR to add the pack indexing macro. I did not add it initially because i was concerned the implementation was not mature enough, but i think we fond most of the bugs.
I think it's fine to set the feature macro for that now with the assumption that we will find all the bugs in the next 6 months.
There seem to be some missing components, like the json node printer.
| FD->setAccess(AS_public); | ||
| Owner->addDecl(FD); |
There was a problem hiding this comment.
Should we call setObjectOfFriendDecl here?
There was a problem hiding this comment.
We don’t do that when we instantiate FriendDecls below, which is why I didn’t add it here.
We already have tests for PCHs, but I can add some more for modules.
We allow
I always forget about something when I add an AST node... |
Following the discussion on #101448 this defines `__cpp_pack_indexing`. Since pack indexing is currently supported in all language modes, the feature test macro is also defined in all language modes.
Following the discussion on llvm#101448 this defines `__cpp_pack_indexing`. Since pack indexing is currently supported in all language modes, the feature test macro is also defined in all language modes. (cherry picked from commit c65afad)
|
So after talking over some of the more horrible edge cases w/ Corentin for a while, it looks like implementing this requires supporting unexpanded packs in nested name specifiers, which we don’t, but he has a patch for that, so I’ll be taking a look at that next. |
cor3ntin
left a comment
cor3ntin
left a comment
There was a problem hiding this comment.
Can you make a separate pr to update the unrelated changes to cxx_status?
I think we are pretty close to be able to land this
Ah, yeah, I didn’t notice those. I’ll get those removed from this pr. |
Also done. I think that’s everything now? |
|
@Sirraide CWG looked at 2917 last week, and changed the status of the issue. Can you revisit your test for it? |
Ah, something that was previously accepted is now disallowed; I’ll implement that tomorrow. |
|
I would recommend creating an issue and see how ewg handles the new information before reaping out a lot of code. |
|
Note that there have been additional discussions happening on the new proposed resolution. |
Hmm, I mean, it looks like they’ve already thought about the consequences though, or am I missing something here, because the case that we currently support and which this ends up disallowing is explicitly listed as disallowed in the core issue:
Ah, that I don’t appear to have access to unfortunately. |
Sure, we don't necessarily need to change our implementation as CWG continues to consider the issue, but we need to update the test and the status for bookkeeping purposes. That's only fair when we write tests against moving targets (open issues). |
In that case, I’ll leave the implementation of this as is for now and just add a FIXME and update the dr status. |
|
I think the dr test already say "Clang 20 implements 2024-07-30 resolution" which is correct |
While I understand what you're referring to (author's claim that the example is nonsense and should be rejected), if you open https://cplusplus.github.io/CWG/issues/2917.html, the only resolution there is dated 2024-08-16. If for whatever reason we think that we should stick to the original claim instead of considering newer proposed resolution, I'm happy with a simple replacement of |
|
the date changed. weird! the resolution did not
…On Fri, Aug 23, 2024, 20:59 Vlad Serebrennikov ***@***.***> wrote:
I think the dr test already say "Clang 20 implements 2024-07-30
resolution" which is correct
While I understand what you're referring to (author's claim that the
example is nonsense and should be rejected), if you open
https://cplusplus.github.io/CWG/issues/2917.html, the only resolution
there is dated 2024-08-16.
If for whatever reason we think that we should stick to that instead of
considering newer proposed resolution, I'm happy with a simple replacement
of open with review in the status.
—
Reply to this email directly, view it on GitHub
<#101448 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKX7632RFGI2VOAS4H3WK3ZS6A77AVCNFSM6AAAAABLZVR77OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBXGY2DMOBVGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I think it did: cplusplus/CWG@4492036 |
|
Pretty sure it did yeah; the change is that this case is now no longer valid: #101448 (comment) |
|
Jens restored the suggested resolution in https://cplusplus.github.io/CWG/issues/2917.html and even put a date on it in case we need to reference it. I also notified him that in our tests we are referring to proposed and suggested resolution by dates. |
|
we are implementing the suggested resolution, which is the first set of change in that issue. we should not implement the "proposed resolution" as discussions are still ongoing (notably concerns of it being too restrictive). |
Well, the suggested resolution, i.e. accepting template<class... Ts>
struct VS {
template<class U>
friend class C<Ts>::Nested...; // now ill-formed
};is what’s already implemented. The proposed resolution rejects this instead. |
6 participants
Implement P2893R3 ‘Variadic friends’ for C++26.
The implementation proper is mostly done, from what I can tell, but I have a few questions, so I thought I’d be best to list them all here (some are for me to investigate, some I’d appreciate it if anyone would happen to know the answer to them):
I don’t know much about theNo longer relevant.ASTImporter, so I’m not entirely sure if I did everything right in there.Is there ever a situation that would cause us to have to instantiate aNo longer relevant.FriendPackDeclin theTemplateDeclInstantiator? We do that forUsingPackDecls, but those are sufficiently different enough from the former that I’m not sure about it. I at least haven’t been able to come up w/ any cases that would cause us to try and instantiate one. (We’ll revisit this later)friend int, long;to be printed like that instead of as separate declarations (which they currently are in the AST). Is that fine or is that too much of a hack? Update: Too much of a hack. Removed.DeclSpec, but I haven’t written too much parser code in Clang before, and wasn’t sure how strict we are wrt putting things in that class because I don’t think the...is part of the specifier according to the grammar. Update: Seems like it’s fine as-is.Thanks also to @zyn0217, who was working on this before me; I’ve taken over working on this since they didn’t have time for it anymore. I’ve incorporated parts of their WIP patch, so I’ll credit them as a co-author whenever this gets merged.
This closes #98587.