Reapply "[Clang] Implement resolution for CWG1835 (#92957, #98547)" #100425
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-coroutines @llvm/pr-subscribers-clang-modules Author: Krystian Stasiowski (sdkrystian) ChangesThis reapplies #92957 and #98547 with the following changes:
template<int I>
struct A {
int x;
};
template<int I>
struct B : A<I> {
void f() {
this->A<I>::f(); // with -std=c++20 and earlier: no error
// with -std=c++23 and later:
// error: no member named 'f' in the global namespace
// error: missing 'template' keyword prior to dependent template name 'A'
}
};Only applying the resolution in C++23 and later conveniently gets around a bug in GCC releases prior to 11.1 that prohibits the use of Patch is 167.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/100425.diff 47 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a34f109ba21ce..1ffc759f7e4b8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -313,6 +313,9 @@ Resolutions to C++ Defect Reports
- Clang now considers ``noexcept(typeid(expr))`` more carefully, instead of always assuming that ``std::bad_typeid`` can be thrown.
(`CWG2191: Incorrect result for noexcept(typeid(v)) <https://cplusplus.github.io/CWG/issues/2191.html>`_).
+- Clang now correctly implements lookup for the terminal name of a member-qualified nested-name-specifier.
+ (`CWG1835: Dependent member lookup before < <https://cplusplus.github.io/CWG/issues/1835.html>`_).
+
C Language Changes
------------------
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index c2feac525c1ea..edaea6fe27cc3 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -3676,9 +3676,9 @@ class CXXUnresolvedConstructExpr final
/// an implicit access if a qualifier is provided.
class CXXDependentScopeMemberExpr final
: public Expr,
- private llvm::TrailingObjects<CXXDependentScopeMemberExpr,
- ASTTemplateKWAndArgsInfo,
- TemplateArgumentLoc, NamedDecl *> {
+ private llvm::TrailingObjects<
+ CXXDependentScopeMemberExpr, NestedNameSpecifierLoc, DeclAccessPair,
+ ASTTemplateKWAndArgsInfo, TemplateArgumentLoc> {
friend class ASTStmtReader;
friend class ASTStmtWriter;
friend TrailingObjects;
@@ -3691,17 +3691,15 @@ class CXXDependentScopeMemberExpr final
/// implicit accesses.
QualType BaseType;
- /// The nested-name-specifier that precedes the member name, if any.
- /// FIXME: This could be in principle store as a trailing object.
- /// However the performance impact of doing so should be investigated first.
- NestedNameSpecifierLoc QualifierLoc;
-
/// The member to which this member expression refers, which
/// can be name, overloaded operator, or destructor.
///
/// FIXME: could also be a template-id
DeclarationNameInfo MemberNameInfo;
+ /// The location of the '->' or '.' operator.
+ SourceLocation OperatorLoc;
+
// CXXDependentScopeMemberExpr is followed by several trailing objects,
// some of which optional. They are in order:
//
@@ -3721,8 +3719,16 @@ class CXXDependentScopeMemberExpr final
return CXXDependentScopeMemberExprBits.HasTemplateKWAndArgsInfo;
}
- bool hasFirstQualifierFoundInScope() const {
- return CXXDependentScopeMemberExprBits.HasFirstQualifierFoundInScope;
+ unsigned getNumUnqualifiedLookups() const {
+ return CXXDependentScopeMemberExprBits.NumUnqualifiedLookups;
+ }
+
+ unsigned numTrailingObjects(OverloadToken<NestedNameSpecifierLoc>) const {
+ return hasQualifier();
+ }
+
+ unsigned numTrailingObjects(OverloadToken<DeclAccessPair>) const {
+ return getNumUnqualifiedLookups();
}
unsigned numTrailingObjects(OverloadToken<ASTTemplateKWAndArgsInfo>) const {
@@ -3733,33 +3739,32 @@ class CXXDependentScopeMemberExpr final
return getNumTemplateArgs();
}
- unsigned numTrailingObjects(OverloadToken<NamedDecl *>) const {
- return hasFirstQualifierFoundInScope();
- }
-
CXXDependentScopeMemberExpr(const ASTContext &Ctx, Expr *Base,
QualType BaseType, bool IsArrow,
SourceLocation OperatorLoc,
NestedNameSpecifierLoc QualifierLoc,
SourceLocation TemplateKWLoc,
- NamedDecl *FirstQualifierFoundInScope,
+ ArrayRef<DeclAccessPair> UnqualifiedLookups,
DeclarationNameInfo MemberNameInfo,
const TemplateArgumentListInfo *TemplateArgs);
- CXXDependentScopeMemberExpr(EmptyShell Empty, bool HasTemplateKWAndArgsInfo,
- bool HasFirstQualifierFoundInScope);
+ CXXDependentScopeMemberExpr(EmptyShell Empty, bool HasQualifier,
+ unsigned NumUnqualifiedLookups,
+ bool HasTemplateKWAndArgsInfo);
public:
static CXXDependentScopeMemberExpr *
Create(const ASTContext &Ctx, Expr *Base, QualType BaseType, bool IsArrow,
SourceLocation OperatorLoc, NestedNameSpecifierLoc QualifierLoc,
- SourceLocation TemplateKWLoc, NamedDecl *FirstQualifierFoundInScope,
+ SourceLocation TemplateKWLoc,
+ ArrayRef<DeclAccessPair> UnqualifiedLookups,
DeclarationNameInfo MemberNameInfo,
const TemplateArgumentListInfo *TemplateArgs);
static CXXDependentScopeMemberExpr *
- CreateEmpty(const ASTContext &Ctx, bool HasTemplateKWAndArgsInfo,
- unsigned NumTemplateArgs, bool HasFirstQualifierFoundInScope);
+ CreateEmpty(const ASTContext &Ctx, bool HasQualifier,
+ unsigned NumUnqualifiedLookups, bool HasTemplateKWAndArgsInfo,
+ unsigned NumTemplateArgs);
/// True if this is an implicit access, i.e. one in which the
/// member being accessed was not written in the source. The source
@@ -3784,34 +3789,35 @@ class CXXDependentScopeMemberExpr final
bool isArrow() const { return CXXDependentScopeMemberExprBits.IsArrow; }
/// Retrieve the location of the '->' or '.' operator.
- SourceLocation getOperatorLoc() const {
- return CXXDependentScopeMemberExprBits.OperatorLoc;
+ SourceLocation getOperatorLoc() const { return OperatorLoc; }
+
+ /// Determines whether this member expression had a nested-name-specifier
+ /// prior to the name of the member, e.g., x->Base::foo.
+ bool hasQualifier() const {
+ return CXXDependentScopeMemberExprBits.HasQualifier;
}
- /// Retrieve the nested-name-specifier that qualifies the member name.
- NestedNameSpecifier *getQualifier() const {
- return QualifierLoc.getNestedNameSpecifier();
+ /// If the member name was qualified, retrieves the nested-name-specifier
+ /// that precedes the member name, with source-location information.
+ NestedNameSpecifierLoc getQualifierLoc() const {
+ if (!hasQualifier())
+ return NestedNameSpecifierLoc();
+ return *getTrailingObjects<NestedNameSpecifierLoc>();
}
- /// Retrieve the nested-name-specifier that qualifies the member
- /// name, with source location information.
- NestedNameSpecifierLoc getQualifierLoc() const { return QualifierLoc; }
+ /// If the member name was qualified, retrieves the
+ /// nested-name-specifier that precedes the member name. Otherwise, returns
+ /// NULL.
+ NestedNameSpecifier *getQualifier() const {
+ return getQualifierLoc().getNestedNameSpecifier();
+ }
- /// Retrieve the first part of the nested-name-specifier that was
- /// found in the scope of the member access expression when the member access
- /// was initially parsed.
- ///
- /// This function only returns a useful result when member access expression
- /// uses a qualified member name, e.g., "x.Base::f". Here, the declaration
- /// returned by this function describes what was found by unqualified name
- /// lookup for the identifier "Base" within the scope of the member access
- /// expression itself. At template instantiation time, this information is
- /// combined with the results of name lookup into the type of the object
- /// expression itself (the class type of x).
- NamedDecl *getFirstQualifierFoundInScope() const {
- if (!hasFirstQualifierFoundInScope())
- return nullptr;
- return *getTrailingObjects<NamedDecl *>();
+ /// Retrieve the declarations found by unqualified lookup for the first
+ /// component name of the nested-name-specifier, if any.
+ ArrayRef<DeclAccessPair> unqualified_lookups() const {
+ if (!getNumUnqualifiedLookups())
+ return std::nullopt;
+ return {getTrailingObjects<DeclAccessPair>(), getNumUnqualifiedLookups()};
}
/// Retrieve the name of the member that this expression refers to.
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 9cd7a364cd3f1..257a61c97c9c6 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -1020,18 +1020,19 @@ class alignas(void *) Stmt {
LLVM_PREFERRED_TYPE(bool)
unsigned IsArrow : 1;
+ /// True if this member expression used a nested-name-specifier to
+ /// refer to the member, e.g., "x->Base::f".
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned HasQualifier : 1;
+
/// Whether this member expression has info for explicit template
/// keyword and arguments.
LLVM_PREFERRED_TYPE(bool)
unsigned HasTemplateKWAndArgsInfo : 1;
- /// See getFirstQualifierFoundInScope() and the comment listing
- /// the trailing objects.
- LLVM_PREFERRED_TYPE(bool)
- unsigned HasFirstQualifierFoundInScope : 1;
-
- /// The location of the '->' or '.' operator.
- SourceLocation OperatorLoc;
+ /// Number of declarations found by unqualified lookup for the
+ /// first component name of the nested-name-specifier.
+ unsigned NumUnqualifiedLookups;
};
class OverloadExprBitfields {
diff --git a/clang/include/clang/AST/UnresolvedSet.h b/clang/include/clang/AST/UnresolvedSet.h
index 1369725ab4e96..ef44499ce5926 100644
--- a/clang/include/clang/AST/UnresolvedSet.h
+++ b/clang/include/clang/AST/UnresolvedSet.h
@@ -97,6 +97,10 @@ class UnresolvedSetImpl {
decls().push_back(DeclAccessPair::make(D, AS));
}
+ void addAllDecls(ArrayRef<DeclAccessPair> Other) {
+ append(iterator(Other.begin()), iterator(Other.end()));
+ }
+
/// Replaces the given declaration with the new one, once.
///
/// \return true if the set changed
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 12aab09f28556..0bd2e35bf2e31 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -895,9 +895,7 @@ def missing_template_arg_list_after_template_kw : Extension<
"keyword">, InGroup<DiagGroup<"missing-template-arg-list-after-template-kw">>,
DefaultError;
-def err_missing_dependent_template_keyword : Error<
- "use 'template' keyword to treat '%0' as a dependent template name">;
-def warn_missing_dependent_template_keyword : ExtWarn<
+def ext_missing_dependent_template_keyword : ExtWarn<
"use 'template' keyword to treat '%0' as a dependent template name">;
def ext_extern_template : Extension<
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 126fea0aef2a7..134bb1b3534e7 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1906,6 +1906,7 @@ class Parser : public CodeCompletionHandler {
}
bool diagnoseUnknownTemplateId(ExprResult TemplateName, SourceLocation Less);
+ bool isMissingTemplateKeywordBeforeScope();
void checkPotentialAngleBracket(ExprResult &PotentialTemplateName);
bool checkPotentialAngleBracketDelimiter(const AngleBracketTracker::Loc &,
const Token &OpToken);
@@ -1966,7 +1967,7 @@ class Parser : public CodeCompletionHandler {
//===--------------------------------------------------------------------===//
// C++ Expressions
ExprResult tryParseCXXIdExpression(CXXScopeSpec &SS, bool isAddressOfOperand,
- Token &Replacement);
+ Token *Replacement = nullptr);
ExprResult tryParseCXXPackIndexingExpression(ExprResult PackIdExpression);
ExprResult ParseCXXPackIndexingExpression(ExprResult PackIdExpression);
@@ -3372,15 +3373,11 @@ class Parser : public CodeCompletionHandler {
BaseResult ParseBaseSpecifier(Decl *ClassDecl);
AccessSpecifier getAccessSpecifierIfPresent() const;
- bool ParseUnqualifiedIdTemplateId(CXXScopeSpec &SS,
- ParsedType ObjectType,
- bool ObjectHadErrors,
- SourceLocation TemplateKWLoc,
- IdentifierInfo *Name,
- SourceLocation NameLoc,
- bool EnteringContext,
- UnqualifiedId &Id,
- bool AssumeTemplateId);
+ bool ParseUnqualifiedIdTemplateId(
+ CXXScopeSpec &SS, ParsedType ObjectType, bool ObjectHadErrors,
+ SourceLocation TemplateKWLoc, SourceLocation TildeLoc,
+ IdentifierInfo *Name, SourceLocation NameLoc, bool EnteringContext,
+ UnqualifiedId &Id, bool AssumeTemplateId);
bool ParseUnqualifiedIdOperator(CXXScopeSpec &SS, bool EnteringContext,
ParsedType ObjectType,
UnqualifiedId &Result);
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index 425b6e2a0b30c..9c22c35535ede 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -75,6 +75,7 @@ class CXXScopeSpec {
SourceRange Range;
NestedNameSpecifierLocBuilder Builder;
ArrayRef<TemplateParameterList *> TemplateParamLists;
+ ArrayRef<DeclAccessPair> UnqualifiedLookups;
public:
SourceRange getRange() const { return Range; }
@@ -91,6 +92,13 @@ class CXXScopeSpec {
return TemplateParamLists;
}
+ void setUnqualifiedLookups(ArrayRef<DeclAccessPair> Found) {
+ UnqualifiedLookups = Found;
+ }
+ ArrayRef<DeclAccessPair> getUnqualifiedLookups() const {
+ return UnqualifiedLookups;
+ }
+
/// Retrieve the representation of the nested-name-specifier.
NestedNameSpecifier *getScopeRep() const {
return Builder.getRepresentation();
diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h
index b0a08a05ac6a0..6b765ef3c980f 100644
--- a/clang/include/clang/Sema/Lookup.h
+++ b/clang/include/clang/Sema/Lookup.h
@@ -483,11 +483,15 @@ class LookupResult {
ResultKind = Found;
}
+ void addAllDecls(ArrayRef<DeclAccessPair> Other) {
+ Decls.addAllDecls(Other);
+ ResultKind = Found;
+ }
+
/// Add all the declarations from another set of lookup
/// results.
void addAllDecls(const LookupResult &Other) {
- Decls.append(Other.Decls.begin(), Other.Decls.end());
- ResultKind = Found;
+ addAllDecls(Other.Decls.pairs());
}
/// Determine whether no result was found because we could not
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 48dff1b76cc57..a86decb67bca1 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2803,7 +2803,8 @@ class Sema final : public SemaBase {
/// (e.g., Base::), perform name lookup for that identifier as a
/// nested-name-specifier within the given scope, and return the result of
/// that name lookup.
- NamedDecl *FindFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS);
+ bool LookupFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS,
+ UnresolvedSetImpl &R);
/// Keeps information about an identifier in a nested-name-spec.
///
@@ -2843,9 +2844,6 @@ class Sema final : public SemaBase {
/// \param EnteringContext If true, enter the context specified by the
/// nested-name-specifier.
/// \param SS Optional nested name specifier preceding the identifier.
- /// \param ScopeLookupResult Provides the result of name lookup within the
- /// scope of the nested-name-specifier that was computed at template
- /// definition time.
/// \param ErrorRecoveryLookup Specifies if the method is called to improve
/// error recovery and what kind of recovery is performed.
/// \param IsCorrectedToColon If not null, suggestion of replace '::' -> ':'
@@ -2854,11 +2852,6 @@ class Sema final : public SemaBase {
/// not '::'.
/// \param OnlyNamespace If true, only considers namespaces in lookup.
///
- /// This routine differs only slightly from ActOnCXXNestedNameSpecifier, in
- /// that it contains an extra parameter \p ScopeLookupResult, which provides
- /// the result of name lookup within the scope of the nested-name-specifier
- /// that was computed at template definition time.
- ///
/// If ErrorRecoveryLookup is true, then this call is used to improve error
/// recovery. This means that it should not emit diagnostics, it should
/// just return true on failure. It also means it should only return a valid
@@ -2867,7 +2860,6 @@ class Sema final : public SemaBase {
/// specifier.
bool BuildCXXNestedNameSpecifier(Scope *S, NestedNameSpecInfo &IdInfo,
bool EnteringContext, CXXScopeSpec &SS,
- NamedDecl *ScopeLookupResult,
bool ErrorRecoveryLookup,
bool *IsCorrectedToColon = nullptr,
bool OnlyNamespace = false);
@@ -8567,11 +8559,12 @@ class Sema final : public SemaBase {
const TemplateArgumentListInfo *TemplateArgs,
bool IsDefiniteInstance, const Scope *S);
- ExprResult ActOnDependentMemberExpr(
- Expr *Base, QualType BaseType, bool IsArrow, SourceLocation OpLoc,
- const CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
- NamedDecl *FirstQualifierInScope, const DeclarationNameInfo &NameInfo,
- const TemplateArgumentListInfo *TemplateArgs);
+ ExprResult
+ ActOnDependentMemberExpr(Expr *Base, QualType BaseType, bool IsArrow,
+ SourceLocation OpLoc, const CXXScopeSpec &SS,
+ SourceLocation TemplateKWLoc,
+ const DeclarationNameInfo &NameInfo,
+ const TemplateArgumentListInfo *TemplateArgs);
/// The main callback when the parser finds something like
/// expression . [nested-name-specifier] identifier
@@ -8627,15 +8620,14 @@ class Sema final : public SemaBase {
ExprResult BuildMemberReferenceExpr(
Expr *Base, QualType BaseType, SourceLocation OpLoc, bool IsArrow,
CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
- NamedDecl *FirstQualifierInScope, const DeclarationNameInfo &NameInfo,
+ const DeclarationNameInfo &NameInfo,
const TemplateArgumentListInfo *TemplateArgs, const Scope *S,
ActOnMemberAccessExtraArgs *ExtraArgs = nullptr);
ExprResult
BuildMemberReferenceExpr(Expr *Base, QualType BaseType, SourceLocation OpLoc,
bool IsArrow, const CXXScopeSpec &SS,
- SourceLocation TemplateKWLoc,
- NamedDecl *FirstQualifierInScope, LookupResult &R,
+ SourceLocation TemplateKWLoc, LookupResult &R,
const TemplateArgumentListInfo *TemplateArgs,
const Scope *S, bool SuppressQualifierCheck = false,
ActOnMemberAccessExtraArgs *ExtraArgs = nullptr);
@@ -11123,15 +11115,14 @@ class Sema final : public SemaBase {
QualType ObjectType, bool EnteringContext,
RequiredTemplateKind RequiredTemplate = SourceLocation(),
AssumedTemplateKind *ATK = nullptr,
- bool AllowTypoCorrection = true);
+ bool AllowTypoCorrection = true, bool MayBeNNS = false);
- TemplateNameKind isTemplateName(Scope *S, CXXScopeSpec &SS,
- bool hasTemplateKeyword,
- const UnqualifiedId &Name,
- ParsedType ObjectType, bool EnteringContext,
- TemplateTy &Template,
- bool &MemberOfUnknownSpecialization,
- bool Disambiguation = false);
+ TemplateNameKind
+ isTemplateName(Scope *S, CXXScopeSpec &SS, bool hasTemplateKeyword,
+ const UnqualifiedId &Name, ParsedType ObjectType,
+ bool EnteringContext, TemplateTy &Template,
+ bool &MemberOfUnknownSpecialization,
+ bool Disambiguation = false, bool MayBeNNS = false);
/// Try to resolve an undeclared templa...
[truncated]
|
|
@llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) ChangesThis reapplies #92957 and #98547 with the following changes:
template<int I>
struct A {
int x;
};
template<int I>
struct B : A<I> {
void f() {
this->A<I>::f(); // with -std=c++20 and earlier: no error
// with -std=c++23 and later:
// error: no member named 'f' in the global namespace
// error: missing 'template' keyword prior to dependent template name 'A'
}
};Only applying the resolution in C++23 and later conveniently gets around a bug in GCC releases prior to 11.1 that prohibits the use of Patch is 167.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/100425.diff 47 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a34f109ba21ce..1ffc759f7e4b8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -313,6 +313,9 @@ Resolutions to C++ Defect Reports
- Clang now considers ``noexcept(typeid(expr))`` more carefully, instead of always assuming that ``std::bad_typeid`` can be thrown.
(`CWG2191: Incorrect result for noexcept(typeid(v)) <https://cplusplus.github.io/CWG/issues/2191.html>`_).
+- Clang now correctly implements lookup for the terminal name of a member-qualified nested-name-specifier.
+ (`CWG1835: Dependent member lookup before < <https://cplusplus.github.io/CWG/issues/1835.html>`_).
+
C Language Changes
------------------
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index c2feac525c1ea..edaea6fe27cc3 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -3676,9 +3676,9 @@ class CXXUnresolvedConstructExpr final
/// an implicit access if a qualifier is provided.
class CXXDependentScopeMemberExpr final
: public Expr,
- private llvm::TrailingObjects<CXXDependentScopeMemberExpr,
- ASTTemplateKWAndArgsInfo,
- TemplateArgumentLoc, NamedDecl *> {
+ private llvm::TrailingObjects<
+ CXXDependentScopeMemberExpr, NestedNameSpecifierLoc, DeclAccessPair,
+ ASTTemplateKWAndArgsInfo, TemplateArgumentLoc> {
friend class ASTStmtReader;
friend class ASTStmtWriter;
friend TrailingObjects;
@@ -3691,17 +3691,15 @@ class CXXDependentScopeMemberExpr final
/// implicit accesses.
QualType BaseType;
- /// The nested-name-specifier that precedes the member name, if any.
- /// FIXME: This could be in principle store as a trailing object.
- /// However the performance impact of doing so should be investigated first.
- NestedNameSpecifierLoc QualifierLoc;
-
/// The member to which this member expression refers, which
/// can be name, overloaded operator, or destructor.
///
/// FIXME: could also be a template-id
DeclarationNameInfo MemberNameInfo;
+ /// The location of the '->' or '.' operator.
+ SourceLocation OperatorLoc;
+
// CXXDependentScopeMemberExpr is followed by several trailing objects,
// some of which optional. They are in order:
//
@@ -3721,8 +3719,16 @@ class CXXDependentScopeMemberExpr final
return CXXDependentScopeMemberExprBits.HasTemplateKWAndArgsInfo;
}
- bool hasFirstQualifierFoundInScope() const {
- return CXXDependentScopeMemberExprBits.HasFirstQualifierFoundInScope;
+ unsigned getNumUnqualifiedLookups() const {
+ return CXXDependentScopeMemberExprBits.NumUnqualifiedLookups;
+ }
+
+ unsigned numTrailingObjects(OverloadToken<NestedNameSpecifierLoc>) const {
+ return hasQualifier();
+ }
+
+ unsigned numTrailingObjects(OverloadToken<DeclAccessPair>) const {
+ return getNumUnqualifiedLookups();
}
unsigned numTrailingObjects(OverloadToken<ASTTemplateKWAndArgsInfo>) const {
@@ -3733,33 +3739,32 @@ class CXXDependentScopeMemberExpr final
return getNumTemplateArgs();
}
- unsigned numTrailingObjects(OverloadToken<NamedDecl *>) const {
- return hasFirstQualifierFoundInScope();
- }
-
CXXDependentScopeMemberExpr(const ASTContext &Ctx, Expr *Base,
QualType BaseType, bool IsArrow,
SourceLocation OperatorLoc,
NestedNameSpecifierLoc QualifierLoc,
SourceLocation TemplateKWLoc,
- NamedDecl *FirstQualifierFoundInScope,
+ ArrayRef<DeclAccessPair> UnqualifiedLookups,
DeclarationNameInfo MemberNameInfo,
const TemplateArgumentListInfo *TemplateArgs);
- CXXDependentScopeMemberExpr(EmptyShell Empty, bool HasTemplateKWAndArgsInfo,
- bool HasFirstQualifierFoundInScope);
+ CXXDependentScopeMemberExpr(EmptyShell Empty, bool HasQualifier,
+ unsigned NumUnqualifiedLookups,
+ bool HasTemplateKWAndArgsInfo);
public:
static CXXDependentScopeMemberExpr *
Create(const ASTContext &Ctx, Expr *Base, QualType BaseType, bool IsArrow,
SourceLocation OperatorLoc, NestedNameSpecifierLoc QualifierLoc,
- SourceLocation TemplateKWLoc, NamedDecl *FirstQualifierFoundInScope,
+ SourceLocation TemplateKWLoc,
+ ArrayRef<DeclAccessPair> UnqualifiedLookups,
DeclarationNameInfo MemberNameInfo,
const TemplateArgumentListInfo *TemplateArgs);
static CXXDependentScopeMemberExpr *
- CreateEmpty(const ASTContext &Ctx, bool HasTemplateKWAndArgsInfo,
- unsigned NumTemplateArgs, bool HasFirstQualifierFoundInScope);
+ CreateEmpty(const ASTContext &Ctx, bool HasQualifier,
+ unsigned NumUnqualifiedLookups, bool HasTemplateKWAndArgsInfo,
+ unsigned NumTemplateArgs);
/// True if this is an implicit access, i.e. one in which the
/// member being accessed was not written in the source. The source
@@ -3784,34 +3789,35 @@ class CXXDependentScopeMemberExpr final
bool isArrow() const { return CXXDependentScopeMemberExprBits.IsArrow; }
/// Retrieve the location of the '->' or '.' operator.
- SourceLocation getOperatorLoc() const {
- return CXXDependentScopeMemberExprBits.OperatorLoc;
+ SourceLocation getOperatorLoc() const { return OperatorLoc; }
+
+ /// Determines whether this member expression had a nested-name-specifier
+ /// prior to the name of the member, e.g., x->Base::foo.
+ bool hasQualifier() const {
+ return CXXDependentScopeMemberExprBits.HasQualifier;
}
- /// Retrieve the nested-name-specifier that qualifies the member name.
- NestedNameSpecifier *getQualifier() const {
- return QualifierLoc.getNestedNameSpecifier();
+ /// If the member name was qualified, retrieves the nested-name-specifier
+ /// that precedes the member name, with source-location information.
+ NestedNameSpecifierLoc getQualifierLoc() const {
+ if (!hasQualifier())
+ return NestedNameSpecifierLoc();
+ return *getTrailingObjects<NestedNameSpecifierLoc>();
}
- /// Retrieve the nested-name-specifier that qualifies the member
- /// name, with source location information.
- NestedNameSpecifierLoc getQualifierLoc() const { return QualifierLoc; }
+ /// If the member name was qualified, retrieves the
+ /// nested-name-specifier that precedes the member name. Otherwise, returns
+ /// NULL.
+ NestedNameSpecifier *getQualifier() const {
+ return getQualifierLoc().getNestedNameSpecifier();
+ }
- /// Retrieve the first part of the nested-name-specifier that was
- /// found in the scope of the member access expression when the member access
- /// was initially parsed.
- ///
- /// This function only returns a useful result when member access expression
- /// uses a qualified member name, e.g., "x.Base::f". Here, the declaration
- /// returned by this function describes what was found by unqualified name
- /// lookup for the identifier "Base" within the scope of the member access
- /// expression itself. At template instantiation time, this information is
- /// combined with the results of name lookup into the type of the object
- /// expression itself (the class type of x).
- NamedDecl *getFirstQualifierFoundInScope() const {
- if (!hasFirstQualifierFoundInScope())
- return nullptr;
- return *getTrailingObjects<NamedDecl *>();
+ /// Retrieve the declarations found by unqualified lookup for the first
+ /// component name of the nested-name-specifier, if any.
+ ArrayRef<DeclAccessPair> unqualified_lookups() const {
+ if (!getNumUnqualifiedLookups())
+ return std::nullopt;
+ return {getTrailingObjects<DeclAccessPair>(), getNumUnqualifiedLookups()};
}
/// Retrieve the name of the member that this expression refers to.
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 9cd7a364cd3f1..257a61c97c9c6 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -1020,18 +1020,19 @@ class alignas(void *) Stmt {
LLVM_PREFERRED_TYPE(bool)
unsigned IsArrow : 1;
+ /// True if this member expression used a nested-name-specifier to
+ /// refer to the member, e.g., "x->Base::f".
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned HasQualifier : 1;
+
/// Whether this member expression has info for explicit template
/// keyword and arguments.
LLVM_PREFERRED_TYPE(bool)
unsigned HasTemplateKWAndArgsInfo : 1;
- /// See getFirstQualifierFoundInScope() and the comment listing
- /// the trailing objects.
- LLVM_PREFERRED_TYPE(bool)
- unsigned HasFirstQualifierFoundInScope : 1;
-
- /// The location of the '->' or '.' operator.
- SourceLocation OperatorLoc;
+ /// Number of declarations found by unqualified lookup for the
+ /// first component name of the nested-name-specifier.
+ unsigned NumUnqualifiedLookups;
};
class OverloadExprBitfields {
diff --git a/clang/include/clang/AST/UnresolvedSet.h b/clang/include/clang/AST/UnresolvedSet.h
index 1369725ab4e96..ef44499ce5926 100644
--- a/clang/include/clang/AST/UnresolvedSet.h
+++ b/clang/include/clang/AST/UnresolvedSet.h
@@ -97,6 +97,10 @@ class UnresolvedSetImpl {
decls().push_back(DeclAccessPair::make(D, AS));
}
+ void addAllDecls(ArrayRef<DeclAccessPair> Other) {
+ append(iterator(Other.begin()), iterator(Other.end()));
+ }
+
/// Replaces the given declaration with the new one, once.
///
/// \return true if the set changed
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 12aab09f28556..0bd2e35bf2e31 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -895,9 +895,7 @@ def missing_template_arg_list_after_template_kw : Extension<
"keyword">, InGroup<DiagGroup<"missing-template-arg-list-after-template-kw">>,
DefaultError;
-def err_missing_dependent_template_keyword : Error<
- "use 'template' keyword to treat '%0' as a dependent template name">;
-def warn_missing_dependent_template_keyword : ExtWarn<
+def ext_missing_dependent_template_keyword : ExtWarn<
"use 'template' keyword to treat '%0' as a dependent template name">;
def ext_extern_template : Extension<
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 126fea0aef2a7..134bb1b3534e7 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1906,6 +1906,7 @@ class Parser : public CodeCompletionHandler {
}
bool diagnoseUnknownTemplateId(ExprResult TemplateName, SourceLocation Less);
+ bool isMissingTemplateKeywordBeforeScope();
void checkPotentialAngleBracket(ExprResult &PotentialTemplateName);
bool checkPotentialAngleBracketDelimiter(const AngleBracketTracker::Loc &,
const Token &OpToken);
@@ -1966,7 +1967,7 @@ class Parser : public CodeCompletionHandler {
//===--------------------------------------------------------------------===//
// C++ Expressions
ExprResult tryParseCXXIdExpression(CXXScopeSpec &SS, bool isAddressOfOperand,
- Token &Replacement);
+ Token *Replacement = nullptr);
ExprResult tryParseCXXPackIndexingExpression(ExprResult PackIdExpression);
ExprResult ParseCXXPackIndexingExpression(ExprResult PackIdExpression);
@@ -3372,15 +3373,11 @@ class Parser : public CodeCompletionHandler {
BaseResult ParseBaseSpecifier(Decl *ClassDecl);
AccessSpecifier getAccessSpecifierIfPresent() const;
- bool ParseUnqualifiedIdTemplateId(CXXScopeSpec &SS,
- ParsedType ObjectType,
- bool ObjectHadErrors,
- SourceLocation TemplateKWLoc,
- IdentifierInfo *Name,
- SourceLocation NameLoc,
- bool EnteringContext,
- UnqualifiedId &Id,
- bool AssumeTemplateId);
+ bool ParseUnqualifiedIdTemplateId(
+ CXXScopeSpec &SS, ParsedType ObjectType, bool ObjectHadErrors,
+ SourceLocation TemplateKWLoc, SourceLocation TildeLoc,
+ IdentifierInfo *Name, SourceLocation NameLoc, bool EnteringContext,
+ UnqualifiedId &Id, bool AssumeTemplateId);
bool ParseUnqualifiedIdOperator(CXXScopeSpec &SS, bool EnteringContext,
ParsedType ObjectType,
UnqualifiedId &Result);
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index 425b6e2a0b30c..9c22c35535ede 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -75,6 +75,7 @@ class CXXScopeSpec {
SourceRange Range;
NestedNameSpecifierLocBuilder Builder;
ArrayRef<TemplateParameterList *> TemplateParamLists;
+ ArrayRef<DeclAccessPair> UnqualifiedLookups;
public:
SourceRange getRange() const { return Range; }
@@ -91,6 +92,13 @@ class CXXScopeSpec {
return TemplateParamLists;
}
+ void setUnqualifiedLookups(ArrayRef<DeclAccessPair> Found) {
+ UnqualifiedLookups = Found;
+ }
+ ArrayRef<DeclAccessPair> getUnqualifiedLookups() const {
+ return UnqualifiedLookups;
+ }
+
/// Retrieve the representation of the nested-name-specifier.
NestedNameSpecifier *getScopeRep() const {
return Builder.getRepresentation();
diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h
index b0a08a05ac6a0..6b765ef3c980f 100644
--- a/clang/include/clang/Sema/Lookup.h
+++ b/clang/include/clang/Sema/Lookup.h
@@ -483,11 +483,15 @@ class LookupResult {
ResultKind = Found;
}
+ void addAllDecls(ArrayRef<DeclAccessPair> Other) {
+ Decls.addAllDecls(Other);
+ ResultKind = Found;
+ }
+
/// Add all the declarations from another set of lookup
/// results.
void addAllDecls(const LookupResult &Other) {
- Decls.append(Other.Decls.begin(), Other.Decls.end());
- ResultKind = Found;
+ addAllDecls(Other.Decls.pairs());
}
/// Determine whether no result was found because we could not
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 48dff1b76cc57..a86decb67bca1 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2803,7 +2803,8 @@ class Sema final : public SemaBase {
/// (e.g., Base::), perform name lookup for that identifier as a
/// nested-name-specifier within the given scope, and return the result of
/// that name lookup.
- NamedDecl *FindFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS);
+ bool LookupFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS,
+ UnresolvedSetImpl &R);
/// Keeps information about an identifier in a nested-name-spec.
///
@@ -2843,9 +2844,6 @@ class Sema final : public SemaBase {
/// \param EnteringContext If true, enter the context specified by the
/// nested-name-specifier.
/// \param SS Optional nested name specifier preceding the identifier.
- /// \param ScopeLookupResult Provides the result of name lookup within the
- /// scope of the nested-name-specifier that was computed at template
- /// definition time.
/// \param ErrorRecoveryLookup Specifies if the method is called to improve
/// error recovery and what kind of recovery is performed.
/// \param IsCorrectedToColon If not null, suggestion of replace '::' -> ':'
@@ -2854,11 +2852,6 @@ class Sema final : public SemaBase {
/// not '::'.
/// \param OnlyNamespace If true, only considers namespaces in lookup.
///
- /// This routine differs only slightly from ActOnCXXNestedNameSpecifier, in
- /// that it contains an extra parameter \p ScopeLookupResult, which provides
- /// the result of name lookup within the scope of the nested-name-specifier
- /// that was computed at template definition time.
- ///
/// If ErrorRecoveryLookup is true, then this call is used to improve error
/// recovery. This means that it should not emit diagnostics, it should
/// just return true on failure. It also means it should only return a valid
@@ -2867,7 +2860,6 @@ class Sema final : public SemaBase {
/// specifier.
bool BuildCXXNestedNameSpecifier(Scope *S, NestedNameSpecInfo &IdInfo,
bool EnteringContext, CXXScopeSpec &SS,
- NamedDecl *ScopeLookupResult,
bool ErrorRecoveryLookup,
bool *IsCorrectedToColon = nullptr,
bool OnlyNamespace = false);
@@ -8567,11 +8559,12 @@ class Sema final : public SemaBase {
const TemplateArgumentListInfo *TemplateArgs,
bool IsDefiniteInstance, const Scope *S);
- ExprResult ActOnDependentMemberExpr(
- Expr *Base, QualType BaseType, bool IsArrow, SourceLocation OpLoc,
- const CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
- NamedDecl *FirstQualifierInScope, const DeclarationNameInfo &NameInfo,
- const TemplateArgumentListInfo *TemplateArgs);
+ ExprResult
+ ActOnDependentMemberExpr(Expr *Base, QualType BaseType, bool IsArrow,
+ SourceLocation OpLoc, const CXXScopeSpec &SS,
+ SourceLocation TemplateKWLoc,
+ const DeclarationNameInfo &NameInfo,
+ const TemplateArgumentListInfo *TemplateArgs);
/// The main callback when the parser finds something like
/// expression . [nested-name-specifier] identifier
@@ -8627,15 +8620,14 @@ class Sema final : public SemaBase {
ExprResult BuildMemberReferenceExpr(
Expr *Base, QualType BaseType, SourceLocation OpLoc, bool IsArrow,
CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
- NamedDecl *FirstQualifierInScope, const DeclarationNameInfo &NameInfo,
+ const DeclarationNameInfo &NameInfo,
const TemplateArgumentListInfo *TemplateArgs, const Scope *S,
ActOnMemberAccessExtraArgs *ExtraArgs = nullptr);
ExprResult
BuildMemberReferenceExpr(Expr *Base, QualType BaseType, SourceLocation OpLoc,
bool IsArrow, const CXXScopeSpec &SS,
- SourceLocation TemplateKWLoc,
- NamedDecl *FirstQualifierInScope, LookupResult &R,
+ SourceLocation TemplateKWLoc, LookupResult &R,
const TemplateArgumentListInfo *TemplateArgs,
const Scope *S, bool SuppressQualifierCheck = false,
ActOnMemberAccessExtraArgs *ExtraArgs = nullptr);
@@ -11123,15 +11115,14 @@ class Sema final : public SemaBase {
QualType ObjectType, bool EnteringContext,
RequiredTemplateKind RequiredTemplate = SourceLocation(),
AssumedTemplateKind *ATK = nullptr,
- bool AllowTypoCorrection = true);
+ bool AllowTypoCorrection = true, bool MayBeNNS = false);
- TemplateNameKind isTemplateName(Scope *S, CXXScopeSpec &SS,
- bool hasTemplateKeyword,
- const UnqualifiedId &Name,
- ParsedType ObjectType, bool EnteringContext,
- TemplateTy &Template,
- bool &MemberOfUnknownSpecialization,
- bool Disambiguation = false);
+ TemplateNameKind
+ isTemplateName(Scope *S, CXXScopeSpec &SS, bool hasTemplateKeyword,
+ const UnqualifiedId &Name, ParsedType ObjectType,
+ bool EnteringContext, TemplateTy &Template,
+ bool &MemberOfUnknownSpecialization,
+ bool Disambiguation = false, bool MayBeNNS = false);
/// Try to resolve an undeclared templa...
[truncated]
|
|
I'm considering whether to add a compatibility warning to ease the transition to the new behavior, e.g. template<int I>
struct A {
int x;
};
template<int I>
struct B : A<I> {
void f() {
this->A<I>::f(); // warning `template` is required after '->' in C++23 and later
}
}; |
Endilll
reviewed
Jul 24, 2024
|
@KyunLFA Could you check if ares compiles with the patch applied using C++23 please? |
|
There is discussion on the Core reflectors about this DR (https://lists.isocpp.org/core/2024/07/16028.php), so given the amount of difficulty we've had with this DR so far, I would recommend we hold off on landing any changes until Core has finished deliberation. |
|
@AaronBallman @cor3ntin @mizvekov So, I've made improvements to our implementation of template<int I>
struct A { int x; };
template<int I, bool J>
struct B { int x; };
template<int I, typename T, int J>
struct C { int x; };
template<int I>
constexpr inline int V = I;
int y;
template<typename T>
void f(T t)
{
t.A<0>::x; // warning: use 'template' keyword to treat 'A' as a dependent template name
t.B<1, true>::x; // warning: use 'template' keyword to treat 'B' as a dependent template name
t.C<2, int, 4>::x; // warning: use 'template' keyword to treat 'C' as a dependent template name
t.A<V<0>>::x; // warning: use 'template' keyword to treat 'A' as a dependent template name
t.B<1, V<1>>::x; // warning: use 'template' keyword to treat 'B' as a dependent template name
t.C<V<2>, int, V<3>>::x; // warning: use 'template' keyword to treat 'C' as a dependent template name
t.A<(1 > 2)>::x; // warning: use 'template' keyword to treat 'A' as a dependent template name
t.A<(1 < 3)>::x; // warning: use 'template' keyword to treat 'A' as a dependent template name
t.A<1 < 4>::x; // error: no member named 'x' in the global namespace
// error: missing 'template' keyword prior to dependent template name 'A'
t.A<1 > 4>::x; // error: no member named 'x' in the global namespace
t.A<0>::y; // ok, parsed as '((t.A) < 0) > ::y'
}This is accomplished by (attempting to) look past the end of the potential template-argument-list, and if the token following the Since we can look past the potential template-argument-list in most cases, we could probably apply CWG1835 in all language modes with minimal impact on existing code. |
That's great, thanks for following up on this! I am still concerned about the double error on the
Yes, I would certainly support that! |
That's trivial to fix, just need to add a note variant of the "missing template" diagnostic. |
mizvekov
reviewed
Jul 28, 2024
clang/lib/Sema/SemaCXXScopeSpec.cpp
Outdated
| @@ -767,6 +757,7 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S, NestedNameSpecInfo &IdInfo, | |||
| else | |||
| Diag(IdInfo.IdentifierLoc, diag::err_undeclared_var_use) | |||
| << IdInfo.Identifier; | |||
| #endif | |||
There was a problem hiding this comment.
You should probably remove these before merging :)
There was a problem hiding this comment.
Yeah, these changes shouldn't really be part of this PR. Just something I noticed when refactoring BuildCXXNestedNameSpecifier that needs to go. I'll address it in another PR.
clang/lib/Sema/SemaCXXScopeSpec.cpp
Outdated
| // If we didn't find anything during our lookup, try again with | ||
| // ordinary name lookup, which can help us produce better error | ||
| // messages. | ||
| if (Found.empty()) { | ||
| Found.clear(LookupOrdinaryName); | ||
| LookupName(Found, S); | ||
| } | ||
| #endif |
sdkrystian
force-pushed
the
reapply-cwg-1835-again
branch
2 times, most recently
from
a3bea01
to
e3829cd
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
I've further improved |
|
Also added support for names prefixed with the |
|
Nice! Please keep track and let us know which parts we haven't seen and have important changes we need to pay more attention to during re-review. Otherwise the patch is getting quite big already :) |
|
With these changes, we can now issue warnings for the missing template<int I>
struct A { int x; };
template<int I, bool J>
struct B { int x; };
template<int I, typename T, int J>
struct C { int x; };
template<typename T>
struct D { int x; };
template<int I>
constexpr inline int V = I;
int y;
template<typename T, int I>
void f(T t) {
t.A<0>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.A<int()>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.A<int(0)>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.A<int*()>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.A<static_cast<D<int>>(0)>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.A<static_cast<const D<int>>(0)>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.B<1, true>::x; // expected-warning {{use 'template' keyword to treat 'B' as a dependent template name}}
t.C<2, int, 4>::x; // expected-warning {{use 'template' keyword to treat 'C' as a dependent template name}}
t.A<V<0>>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.B<1, V<1>>::x; // expected-warning {{use 'template' keyword to treat 'B' as a dependent template name}}
t.C<V<2>, int, V<3>>::x; // expected-warning {{use 'template' keyword to treat 'C' as a dependent template name}}
t.A<1 && T::template f<0>()>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.A<(1 > 2)>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.A<(1 < 3)>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.B<A<I>::x < int, 1>::x; // expected-warning {{use 'template' keyword to treat 'x' as a dependent template name}}
t.B<A<I>::x < 0, 1>::x; // expected-error {{no member named 'x' in the global namespace}}
// expected-error {{missing 'template' keyword prior to dependent template name 'B'}}
t.A<1 < 4>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.A<I < 4>::x; // expected-error {{no member named 'x' in the global namespace}}
// expected-error {{missing 'template' keyword prior to dependent template name 'A'}}
t.A<I ? I < 4 : false>::x; // expected-warning {{use 'template' keyword to treat 'A' as a dependent template name}}
t.A<1 > 4>::x; // expected-error {{no member named 'x' in the global namespace}}
t.A<0>::y; // ok, parsed as '((t.A) < 0) > ::y'
}Would like to get feedback from @AaronBallman, @cor3ntin, @hokein, and perhaps @zygoloid on this :). I highly doubt the discussion on the reflector will result in any mitigation mechanisms being added (see my reply here). If any action is taken, the most I foresee would be applying it in C++23 & later. |
zygoloid
reviewed
Jul 30, 2024
|
There is now https://cplusplus.github.io/CWG/issues/2920.html |
|
CWG reviewed CWG2920 during the 08-16 teleconference:
The meeting minutes portray a general feeling of apathy from CWG for code broken by the resolution. With the enhanced detection and recovery from missing |
|
In a follow up, the following suggestion was made
This seems worth exploring to me |
|
@cor3ntin This wouldn't fix the case that abseil users complained about (example reduced from the source): template <typename... Ts>
class CompressedTuple
: private internal_compressed_tuple::CompressedTupleImpl<
CompressedTuple<Ts...>, absl::index_sequence_for<Ts...>,
internal_compressed_tuple::ShouldAnyUseBase<Ts...>()> {
template <int I>
using StorageT = internal_compressed_tuple::Storage<ElemT<I>, I>;
public:
template <int I>
constexpr ElemT<I>&& get() && {
return std::move(*this).StorageT<I>::get();
}
};Moreover, this fails to address cases where the base class declares a non-static data member with the same name as the class template: template<typename T>
struct A
{
int A;
};
template<typename T>
struct B : A<T>
{
bool f()
{
return this->A < 1; // '<' would be interpreted as the start of a template argument list
}
};I think that implementing CWG1835's resolution as it stands is our best option. Doing so results in the most consistent behavior (e.g. 'use template when the object expression is dependent'), the most obvious fixes to achieve the intended interpretation (i.e. 'add |
My reading of Core's minutes isn't that there's apathy but more that there's not an obvious solution that resolves all situations, and therefore there's a call for a paper to be written. I don't agree with the logic that there's a need to identify a fix before deciding whether something is or is not a DR -- if it breaks existing code and we don't know how to correct it, making it not a DR has the least fallout. So if we move forward, I think the best approach is to apply it to C++23 and up. However, it's not clear to me why we need to implement this right now instead of trying to determine what the best possible compromise is for our needs and writing the paper CWG is asking for. |
sdkrystian
force-pushed
the
reapply-cwg-1835-again
branch
from
4cc02b0
to
ec0fc1e
Compare
sdkrystian
force-pushed
the
reapply-cwg-1835-again
branch
from
ec0fc1e
to
80061b1
Compare
sdkrystian
force-pushed
the
reapply-cwg-1835-again
branch
2 times, most recently
from
4db019f
to
cfff947
Compare
…nt class member access
…when skipping past template arguments
sdkrystian
force-pushed
the
reapply-cwg-1835-again
branch
from
cfff947
to
8c68600
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reapplies #92957 and #98547 with the following changes:
We only apply the resolution to CWG1835 in C++23 and later, andOnly applying the resolution in C++23 and later conveniently gets around a bug in GCC releases prior to 11.1 that prohibits the use oftemplateprior to the nested-name-specifier in a class member access expression as the affected versions do not support C++23.