Revert "Reapply "[Clang] Implement resolution for CWG1835 (#92957)" (#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)

Author: hokein

clang/docs/ReleaseNotes.rst

@@ -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
 ------------------
 

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, NestedNameSpecifierLoc, DeclAccessPair,
-          ASTTemplateKWAndArgsInfo, TemplateArgumentLoc> {
+      private llvm::TrailingObjects<CXXDependentScopeMemberExpr,
+                                    ASTTemplateKWAndArgsInfo,
+                                    TemplateArgumentLoc, NamedDecl *> {
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
   friend TrailingObjects;
@@ -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:
   //
@@ -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 {
@@ -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
@@ -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.

clang/include/clang/AST/Stmt.h

@@ -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 {

clang/include/clang/AST/UnresolvedSet.h

@@ -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

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -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>;

clang/include/clang/Parse/Parser.h

@@ -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);

clang/include/clang/Sema/DeclSpec.h

@@ -75,7 +75,6 @@ class CXXScopeSpec {
   SourceRange Range;
   NestedNameSpecifierLocBuilder Builder;
   ArrayRef<TemplateParameterList *> TemplateParamLists;
-  ArrayRef<DeclAccessPair> UnqualifiedLookups;
 
 public:
   SourceRange getRange() const { return Range; }
@@ -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();

clang/include/clang/Sema/Lookup.h

@@ -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

clang/include/clang/Sema/Sema.h

@@ -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.
   ///
@@ -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 '::' -> ':'
@@ -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
@@ -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);
@@ -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
@@ -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);
@@ -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.
   ///
@@ -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,