Skip to content

Commit

Permalink
Revert "Reapply "[Clang] Implement resolution for CWG1835 (#92957)" (#…
Browse files Browse the repository at this point in the history
…98547)"

This reverts commit ce4aada and a
follow-up patch 8ef26f1.

This new warning can not be fully suppressed by the
`-Wno-missing-dependent-template-keyword` flag, this gives developer no
time to do the cleanup in a large codebase, see #98547 (comment)
  • Loading branch information
hokein committed Jul 15, 2024
1 parent 6484655 commit 59e56ee
Show file tree
Hide file tree
Showing 47 changed files with 815 additions and 1,121 deletions.
4 changes: 0 additions & 4 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,6 @@ 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>`_).
The warning can be disabled via `-Wno-missing-dependent-template-keyword`.

C Language Changes
------------------

Expand Down
92 changes: 43 additions & 49 deletions clang/include/clang/AST/ExprCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -3676,9 +3676,9 @@ class CXXUnresolvedConstructExpr final
/// an implicit access if a qualifier is provided.
class CXXDependentScopeMemberExpr final
: public Expr,
private llvm::TrailingObjects<
CXXDependentScopeMemberExpr, NestedNameSpecifierLoc, DeclAccessPair,
ASTTemplateKWAndArgsInfo, TemplateArgumentLoc> {
private llvm::TrailingObjects<CXXDependentScopeMemberExpr,
ASTTemplateKWAndArgsInfo,
TemplateArgumentLoc, NamedDecl *> {
friend class ASTStmtReader;
friend class ASTStmtWriter;
friend TrailingObjects;
Expand All @@ -3691,15 +3691,17 @@ 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:
//
Expand All @@ -3719,16 +3721,8 @@ class CXXDependentScopeMemberExpr final
return CXXDependentScopeMemberExprBits.HasTemplateKWAndArgsInfo;
}

unsigned getNumUnqualifiedLookups() const {
return CXXDependentScopeMemberExprBits.NumUnqualifiedLookups;
}

unsigned numTrailingObjects(OverloadToken<NestedNameSpecifierLoc>) const {
return hasQualifier();
}

unsigned numTrailingObjects(OverloadToken<DeclAccessPair>) const {
return getNumUnqualifiedLookups();
bool hasFirstQualifierFoundInScope() const {
return CXXDependentScopeMemberExprBits.HasFirstQualifierFoundInScope;
}

unsigned numTrailingObjects(OverloadToken<ASTTemplateKWAndArgsInfo>) const {
Expand All @@ -3739,32 +3733,33 @@ 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,
ArrayRef<DeclAccessPair> UnqualifiedLookups,
NamedDecl *FirstQualifierFoundInScope,
DeclarationNameInfo MemberNameInfo,
const TemplateArgumentListInfo *TemplateArgs);

CXXDependentScopeMemberExpr(EmptyShell Empty, bool HasQualifier,
unsigned NumUnqualifiedLookups,
bool HasTemplateKWAndArgsInfo);
CXXDependentScopeMemberExpr(EmptyShell Empty, bool HasTemplateKWAndArgsInfo,
bool HasFirstQualifierFoundInScope);

public:
static CXXDependentScopeMemberExpr *
Create(const ASTContext &Ctx, Expr *Base, QualType BaseType, bool IsArrow,
SourceLocation OperatorLoc, NestedNameSpecifierLoc QualifierLoc,
SourceLocation TemplateKWLoc,
ArrayRef<DeclAccessPair> UnqualifiedLookups,
SourceLocation TemplateKWLoc, NamedDecl *FirstQualifierFoundInScope,
DeclarationNameInfo MemberNameInfo,
const TemplateArgumentListInfo *TemplateArgs);

static CXXDependentScopeMemberExpr *
CreateEmpty(const ASTContext &Ctx, bool HasQualifier,
unsigned NumUnqualifiedLookups, bool HasTemplateKWAndArgsInfo,
unsigned NumTemplateArgs);
CreateEmpty(const ASTContext &Ctx, bool HasTemplateKWAndArgsInfo,
unsigned NumTemplateArgs, bool HasFirstQualifierFoundInScope);

/// True if this is an implicit access, i.e. one in which the
/// member being accessed was not written in the source. The source
Expand All @@ -3789,35 +3784,34 @@ class CXXDependentScopeMemberExpr final
bool isArrow() const { return CXXDependentScopeMemberExprBits.IsArrow; }

/// Retrieve the location of the '->' or '.' operator.
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;
}

/// 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>();
SourceLocation getOperatorLoc() const {
return CXXDependentScopeMemberExprBits.OperatorLoc;
}

/// If the member name was qualified, retrieves the
/// nested-name-specifier that precedes the member name. Otherwise, returns
/// NULL.
/// Retrieve the nested-name-specifier that qualifies the member name.
NestedNameSpecifier *getQualifier() const {
return getQualifierLoc().getNestedNameSpecifier();
return QualifierLoc.getNestedNameSpecifier();
}

/// 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 nested-name-specifier that qualifies the member
/// name, with source location information.
NestedNameSpecifierLoc getQualifierLoc() const { return QualifierLoc; }

/// 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 name of the member that this expression refers to.
Expand Down
15 changes: 7 additions & 8 deletions clang/include/clang/AST/Stmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -1020,19 +1020,18 @@ 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;

/// Number of declarations found by unqualified lookup for the
/// first component name of the nested-name-specifier.
unsigned NumUnqualifiedLookups;
/// See getFirstQualifierFoundInScope() and the comment listing
/// the trailing objects.
LLVM_PREFERRED_TYPE(bool)
unsigned HasFirstQualifierFoundInScope : 1;

/// The location of the '->' or '.' operator.
SourceLocation OperatorLoc;
};

class OverloadExprBitfields {
Expand Down
4 changes: 0 additions & 4 deletions clang/include/clang/AST/UnresolvedSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,6 @@ 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
Expand Down
7 changes: 4 additions & 3 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -895,9 +895,10 @@ def missing_template_arg_list_after_template_kw : Extension<
"keyword">, InGroup<DiagGroup<"missing-template-arg-list-after-template-kw">>,
DefaultError;

def ext_missing_dependent_template_keyword : ExtWarn<
"use 'template' keyword to treat '%0' as a dependent template name">,
InGroup<DiagGroup<"missing-dependent-template-keyword">>;
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<
"use 'template' keyword to treat '%0' as a dependent template name">;

def ext_extern_template : Extension<
"extern templates are a C++11 extension">, InGroup<CXX11>;
Expand Down
14 changes: 9 additions & 5 deletions clang/include/clang/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -3372,11 +3372,15 @@ class Parser : public CodeCompletionHandler {
BaseResult ParseBaseSpecifier(Decl *ClassDecl);
AccessSpecifier getAccessSpecifierIfPresent() const;

bool ParseUnqualifiedIdTemplateId(
CXXScopeSpec &SS, ParsedType ObjectType, bool ObjectHadErrors,
SourceLocation TemplateKWLoc, SourceLocation TildeLoc,
IdentifierInfo *Name, SourceLocation NameLoc, bool EnteringContext,
UnqualifiedId &Id, bool AssumeTemplateId);
bool ParseUnqualifiedIdTemplateId(CXXScopeSpec &SS,
ParsedType ObjectType,
bool ObjectHadErrors,
SourceLocation TemplateKWLoc,
IdentifierInfo *Name,
SourceLocation NameLoc,
bool EnteringContext,
UnqualifiedId &Id,
bool AssumeTemplateId);
bool ParseUnqualifiedIdOperator(CXXScopeSpec &SS, bool EnteringContext,
ParsedType ObjectType,
UnqualifiedId &Result);
Expand Down
8 changes: 0 additions & 8 deletions clang/include/clang/Sema/DeclSpec.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ class CXXScopeSpec {
SourceRange Range;
NestedNameSpecifierLocBuilder Builder;
ArrayRef<TemplateParameterList *> TemplateParamLists;
ArrayRef<DeclAccessPair> UnqualifiedLookups;

public:
SourceRange getRange() const { return Range; }
Expand All @@ -92,13 +91,6 @@ 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();
Expand Down
8 changes: 2 additions & 6 deletions clang/include/clang/Sema/Lookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -483,15 +483,11 @@ 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) {
addAllDecls(Other.Decls.pairs());
Decls.append(Other.Decls.begin(), Other.Decls.end());
ResultKind = Found;
}

/// Determine whether no result was found because we could not
Expand Down
54 changes: 32 additions & 22 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -2803,8 +2803,7 @@ 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.
bool LookupFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS,
UnresolvedSetImpl &R);
NamedDecl *FindFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS);

/// Keeps information about an identifier in a nested-name-spec.
///
Expand Down Expand Up @@ -2844,6 +2843,9 @@ 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 '::' -> ':'
Expand All @@ -2852,6 +2854,11 @@ 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
Expand All @@ -2860,6 +2867,7 @@ 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);
Expand Down Expand Up @@ -8559,12 +8567,11 @@ 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,
const DeclarationNameInfo &NameInfo,
const TemplateArgumentListInfo *TemplateArgs);
ExprResult ActOnDependentMemberExpr(
Expr *Base, QualType BaseType, bool IsArrow, SourceLocation OpLoc,
const CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
NamedDecl *FirstQualifierInScope, const DeclarationNameInfo &NameInfo,
const TemplateArgumentListInfo *TemplateArgs);

/// The main callback when the parser finds something like
/// expression . [nested-name-specifier] identifier
Expand Down Expand Up @@ -8620,14 +8627,15 @@ class Sema final : public SemaBase {
ExprResult BuildMemberReferenceExpr(
Expr *Base, QualType BaseType, SourceLocation OpLoc, bool IsArrow,
CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
const DeclarationNameInfo &NameInfo,
NamedDecl *FirstQualifierInScope, 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, LookupResult &R,
SourceLocation TemplateKWLoc,
NamedDecl *FirstQualifierInScope, LookupResult &R,
const TemplateArgumentListInfo *TemplateArgs,
const Scope *S, bool SuppressQualifierCheck = false,
ActOnMemberAccessExtraArgs *ExtraArgs = nullptr);
Expand Down Expand Up @@ -11115,14 +11123,15 @@ class Sema final : public SemaBase {
QualType ObjectType, bool EnteringContext,
RequiredTemplateKind RequiredTemplate = SourceLocation(),
AssumedTemplateKind *ATK = nullptr,
bool AllowTypoCorrection = true, bool MayBeNNS = false);
bool AllowTypoCorrection = true);

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);
TemplateNameKind isTemplateName(Scope *S, CXXScopeSpec &SS,
bool hasTemplateKeyword,
const UnqualifiedId &Name,
ParsedType ObjectType, bool EnteringContext,
TemplateTy &Template,
bool &MemberOfUnknownSpecialization,
bool Disambiguation = false);

/// Try to resolve an undeclared template name as a type template.
///
Expand Down Expand Up @@ -11455,11 +11464,12 @@ class Sema final : public SemaBase {
/// For example, given "x.MetaFun::template apply", the scope specifier
/// \p SS will be "MetaFun::", \p TemplateKWLoc contains the location
/// of the "template" keyword, and "apply" is the \p Name.
TemplateNameKind
ActOnTemplateName(Scope *S, CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
const UnqualifiedId &Name, ParsedType ObjectType,
bool EnteringContext, TemplateTy &Template,
bool AllowInjectedClassName = false, bool MayBeNNS = false);
TemplateNameKind ActOnTemplateName(Scope *S, CXXScopeSpec &SS,
SourceLocation TemplateKWLoc,
const UnqualifiedId &Name,
ParsedType ObjectType,
bool EnteringContext, TemplateTy &Template,
bool AllowInjectedClassName = false);

DeclResult ActOnClassTemplateSpecialization(
Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,
Expand Down
Loading

0 comments on commit 59e56ee

Please sign in to comment.