[Clang] Implement resolution for CWG1835 (#92957)

CWG1835 was one of the many core issues resolved by P1787R6: "Declarations and where to
find them" (http://wg21.link/p1787r6). Its resolution changes how
member-qualified names (as defined by [basic.lookup.qual.general] p2) are looked
up. This patch implementation that resolution.

Previously, an _identifier_ following `.` or `->` would be first looked
up in the type of the object expression (i.e. qualified lookup), and
then in the context of the _postfix-expression_ (i.e. unqualified
lookup) if nothing was found; the result of the second lookup was
required to name a class template. Notably, this second lookup would
occur even when the object expression was dependent, and its result
would be used to determine whether a `<` token is the start of a
_template-argument_list_.

The new wording in [basic.lookup.qual.general] p2 states:
> A member-qualified name is the (unique) component name, if any, of
> - an _unqualified-id_ or
> - a _nested-name-specifier_ of the form _`type-name ::`_ or
_`namespace-name ::`​_
>
> in the id-expression of a class member access expression. A
***qualified name*** is
> - a member-qualified name or
> - the terminal name of
>     - a _qualified-id_,
>     - a _using-declarator_,
>     - a _typename-specifier_,
>     - a _qualified-namespace-specifier_, or
> - a _nested-name-specifier_, _elaborated-type-specifier_, or
_class-or-decltype_ that has a _nested-name-specifier_.
>
> The _lookup context_ of a member-qualified name is the type of its
associated object expression (considered dependent if the object
expression is type-dependent). The lookup context of any other qualified
name is the type, template, or namespace nominated by the preceding
_nested-name-specifier_.

And [basic.lookup.qual.general] p3 now states:
> _Qualified name lookup_ in a class, namespace, or enumeration performs
a search of the scope associated with it except as specified below.
Unless otherwise specified, a qualified name undergoes qualified name
lookup in its lookup context from the point where it appears unless the
lookup context either is dependent and is not the current instantiation
or is not a class or class template. If nothing is found by qualified
lookup for a member-qualified name that is the terminal name of a
_nested-name-specifier_ and is not dependent, it undergoes unqualified
lookup.

In non-standardese terms, these two paragraphs essentially state the
following:
- A name that immediately follows `.` or `->` in a class member access
expression is a member-qualified name
- A member-qualified name will be first looked up in the type of the
object expression `T` unless `T` is a dependent type that is _not_ the
current instantiation, e.g.
```
template<typename T>
struct A
{
    void f(T* t)
    {
        this->x; // type of the object expression is 'A<T>'. although 'A<T>' is dependent, it is the
                 // current instantiation so we look up 'x' in the template definition context.
        
        t->y; // type of the object expression is 'T' ('->' is transformed to '.' per [expr.ref]). 
              // 'T' is dependent and is *not* the current instantiation, so we lookup 'y' in the 
              // template instantiation context.
    }
};
```
- If the first lookup finds nothing and:
- the member-qualified name is the first component of a
_nested-name-specifier_ (which could be an _identifier_ or a
_simple-template-id_), and either:
- the type of the object expression is the current instantiation and it
has no dependent base classes, or
        - the type of the object expression is not dependent

  then we lookup the name again, this time via unqualified lookup.

Although the second (unqualified) lookup is stated not to occur when the
member-qualified name is dependent, a dependent name will _not_ be
dependent once the template is instantiated, so the second lookup must
"occur" during instantiation if qualified lookup does not find anything.
This means that we must perform the second (unqualified) lookup during
parsing even when the type of the object expression is dependent, but
those results are _not_ used to determine whether a `<` token is the
start of a _template-argument_list_; they are stored so we can replicate
the second lookup during instantiation.

In even simpler terms (paraphrasing the meeting minutes from the review of P1787; see https://wiki.edg.com/bin/view/Wg21summer2020/P1787%28Lookup%29Review2020-06-15Through2020-06-18):
- Unqualified lookup always happens for the first name in a
_nested-name-specifier_ that follows `.` or `->`
- The result of that lookup is only used to determine whether `<` is the
start of a _template-argument-list_ if the first (qualified) lookup
found nothing and the lookup context:
    - is not dependent, or 
    - is the current instantiation and has no dependent base classes.

An example:
```
struct A 
{
     void f();
};

template<typename T>
using B = A;

template<typename T>
struct C : A
{
    template<typename U>
    void g();

    void h(T* t)
    {
        this->g<int>(); // ok, '<' is the start of a template-argument-list ('g' was found via qualified lookup in the current instantiation)
        this->B<void>::f(); // ok, '<' is the start of a template-argument-list (current instantiation has no dependent bases, 'B' was found via unqualified lookup)
        t->g<int>(); // error: '<' means less than (unqualified lookup does not occur for a member-qualified name that isn't the first component of a nested-name-specifier)
        t->B<void>::f(); // error: '<' means less than (unqualified lookup does not occur if the name is dependent)
        t->template B<void>::f(); // ok: '<' is the start of a template-argument-list ('template' keyword used)
    }
};
```

Some additional notes:
- Per [basic.lookup.qual.general] p1, lookup for a
member-qualified name only considers namespaces, types, and templates
whose specializations are types if it's an _identifier_ followed by
`::`; lookup for the component name of a _simple-template-id_ followed
by `::` is _not_ subject to this rule.
- The wording which specifies when the second unqualified lookup occurs
appears to be paradoxical. We are supposed to do it only for the first
component name of a _nested-name-specifier_ that follows `.` or `->`
when qualified lookup finds nothing. However, when that name is followed
by `<` (potentially starting a _simple-template-id_) we don't _know_
whether it will be the start of a _nested-name-specifier_ until we do
the lookup -- but we aren't supposed to do the lookup until we know it's
part of a _nested-name-specifier_! ***However***, since we only do the
second lookup when the first lookup finds nothing (and the name isn't
dependent), ***and*** since neither lookup is type-only, the only valid
option is for the name to be the _template-name_ in a
_simple-template-id_ that is followed by `::` (it can't be an
_unqualified-id_ naming a member because we already determined that the
lookup context doesn't have a member with that name). Thus, we can lock
into the _nested-name-specifier_ interpretation and do the second lookup
without having to know whether the _simple-template-id_ will be followed
by `::` yet.

1 files changed, 33 insertions, 41 deletions

diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 2070f3b..8519618 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -552,11 +552,9 @@ static Decl *FindGetterSetterNameDecl(const ObjCObjectPointerType *QIdTy,
 }
 
 ExprResult
-Sema::ActOnDependentMemberExpr(Expr *BaseExpr, QualType BaseType,
-                               bool IsArrow, SourceLocation OpLoc,
-                               const CXXScopeSpec &SS,
+Sema::ActOnDependentMemberExpr(Expr *BaseExpr, QualType BaseType, bool IsArrow,
+                               SourceLocation OpLoc, const CXXScopeSpec &SS,
                                SourceLocation TemplateKWLoc,
-                               NamedDecl *FirstQualifierInScope,
                                const DeclarationNameInfo &NameInfo,
                                const TemplateArgumentListInfo *TemplateArgs) {
   // Even in dependent contexts, try to diagnose base expressions with
@@ -590,8 +588,8 @@ Sema::ActOnDependentMemberExpr(Expr *BaseExpr, QualType BaseType,
   // must have pointer type, and the accessed type is the pointee.
   return CXXDependentScopeMemberExpr::Create(
       Context, BaseExpr, BaseType, IsArrow, OpLoc,
-      SS.getWithLocInContext(Context), TemplateKWLoc, FirstQualifierInScope,
-      NameInfo, TemplateArgs);
+      SS.getWithLocInContext(Context), TemplateKWLoc,
+      SS.getUnqualifiedLookups(), NameInfo, TemplateArgs);
 }
 
 /// We know that the given qualified member reference points only to
@@ -767,8 +765,9 @@ static bool LookupMemberExprInRecord(Sema &SemaRef, LookupResult &R,
           R.addDecl(ND);
         R.resolveKind();
         return SemaRef.BuildMemberReferenceExpr(
-            BaseExpr, BaseExpr->getType(), OpLoc, IsArrow, SS, SourceLocation(),
-            nullptr, R, nullptr, nullptr);
+            BaseExpr, BaseExpr->getType(), OpLoc, IsArrow, SS,
+            /*TemplateKWLoc=*/SourceLocation(), R, /*TemplateArgs=*/nullptr,
+            /*S=*/nullptr);
       },
       Sema::CTK_ErrorRecovery, DC);
 
@@ -784,7 +783,7 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
 ExprResult Sema::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) {
   LookupResult R(*this, NameInfo, LookupMemberName);
@@ -828,10 +827,9 @@ ExprResult Sema::BuildMemberReferenceExpr(
   if (SS.isInvalid())
     return ExprError();
 
-  return BuildMemberReferenceExpr(Base, BaseType,
-                                  OpLoc, IsArrow, SS, TemplateKWLoc,
-                                  FirstQualifierInScope, R, TemplateArgs, S,
-                                  false, ExtraArgs);
+  return BuildMemberReferenceExpr(Base, BaseType, OpLoc, IsArrow, SS,
+                                  TemplateKWLoc, R, TemplateArgs, S,
+                                  /*SuppressQualifierCheck=*/false, ExtraArgs);
 }
 
 ExprResult
@@ -969,17 +967,11 @@ static bool IsInFnTryBlockHandler(const Scope *S) {
   return false;
 }
 
-ExprResult
-Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
-                               SourceLocation OpLoc, bool IsArrow,
-                               const CXXScopeSpec &SS,
-                               SourceLocation TemplateKWLoc,
-                               NamedDecl *FirstQualifierInScope,
-                               LookupResult &R,
-                               const TemplateArgumentListInfo *TemplateArgs,
-                               const Scope *S,
-                               bool SuppressQualifierCheck,
-                               ActOnMemberAccessExtraArgs *ExtraArgs) {
+ExprResult Sema::BuildMemberReferenceExpr(
+    Expr *BaseExpr, QualType BaseExprType, SourceLocation OpLoc, bool IsArrow,
+    const CXXScopeSpec &SS, SourceLocation TemplateKWLoc, LookupResult &R,
+    const TemplateArgumentListInfo *TemplateArgs, const Scope *S,
+    bool SuppressQualifierCheck, ActOnMemberAccessExtraArgs *ExtraArgs) {
   assert(!SS.isInvalid() && "nested-name-specifier cannot be invalid");
   // If the member wasn't found in the current instantiation, or if the
   // arrow operator was used with a dependent non-pointer object expression,
@@ -989,8 +981,8 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
        (SS.isSet() ? SS.getScopeRep()->isDependent()
                    : BaseExprType->isDependentType())))
     return ActOnDependentMemberExpr(BaseExpr, BaseExprType, IsArrow, OpLoc, SS,
-                                    TemplateKWLoc, FirstQualifierInScope,
-                                    R.getLookupNameInfo(), TemplateArgs);
+                                    TemplateKWLoc, R.getLookupNameInfo(),
+                                    TemplateArgs);
 
   QualType BaseType = BaseExprType;
   if (IsArrow) {
@@ -1195,9 +1187,9 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
 
     // Non-dependent member, but dependent template arguments.
     if (!VDecl.get())
-      return ActOnDependentMemberExpr(
-          BaseExpr, BaseExpr->getType(), IsArrow, OpLoc, SS, TemplateKWLoc,
-          FirstQualifierInScope, MemberNameInfo, TemplateArgs);
+      return ActOnDependentMemberExpr(BaseExpr, BaseExpr->getType(), IsArrow,
+                                      OpLoc, SS, TemplateKWLoc, MemberNameInfo,
+                                      TemplateArgs);
 
     VarDecl *Var = cast<VarDecl>(VDecl.get());
     if (!Var->getTemplateSpecializationKind())
@@ -1763,15 +1755,16 @@ ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr *Base,
   const TemplateArgumentListInfo *TemplateArgs;
   DecomposeUnqualifiedId(Id, TemplateArgsBuffer,
                          NameInfo, TemplateArgs);
-
-  bool IsArrow = (OpKind == tok::arrow);
+  bool IsArrow = OpKind == tok::arrow;
 
   if (getLangOpts().HLSL && IsArrow)
     return ExprError(Diag(OpLoc, diag::err_hlsl_operator_unsupported) << 2);
 
-  NamedDecl *FirstQualifierInScope
-    = (!SS.isSet() ? nullptr : FindFirstQualifierInScope(S, SS.getScopeRep()));
-
+  UnresolvedSet<4> UnqualifiedLookups;
+  if (SS.isValid() &&
+      LookupFirstQualifierInScope(S, SS.getScopeRep(), UnqualifiedLookups)) {
+    SS.setUnqualifiedLookups(UnqualifiedLookups.pairs());
+  }
   // This is a postfix expression, so get rid of ParenListExprs.
   ExprResult Result = MaybeConvertParenListExprToParenExpr(S, Base);
   if (Result.isInvalid()) return ExprError();
@@ -1779,8 +1772,8 @@ ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr *Base,
 
   ActOnMemberAccessExtraArgs ExtraArgs = {S, Id, ObjCImpDecl};
   ExprResult Res = BuildMemberReferenceExpr(
-      Base, Base->getType(), OpLoc, IsArrow, SS, TemplateKWLoc,
-      FirstQualifierInScope, NameInfo, TemplateArgs, S, &ExtraArgs);
+      Base, Base->getType(), OpLoc, IsArrow, SS, TemplateKWLoc, NameInfo,
+      TemplateArgs, S, &ExtraArgs);
 
   if (!Res.isInvalid() && isa<MemberExpr>(Res.get()))
     CheckMemberAccessOfNoDeref(cast<MemberExpr>(Res.get()));
@@ -1924,9 +1917,8 @@ Sema::BuildImplicitMemberExpr(const CXXScopeSpec &SS,
     baseExpr = BuildCXXThisExpr(loc, ThisTy, /*IsImplicit=*/true);
   }
 
-  return BuildMemberReferenceExpr(
-      baseExpr, ThisTy,
-      /*OpLoc=*/SourceLocation(),
-      /*IsArrow=*/!getLangOpts().HLSL, SS, TemplateKWLoc,
-      /*FirstQualifierInScope=*/nullptr, R, TemplateArgs, S);
+  return BuildMemberReferenceExpr(baseExpr, ThisTy,
+                                  /*OpLoc=*/SourceLocation(),
+                                  /*IsArrow=*/!getLangOpts().HLSL, SS,
+                                  TemplateKWLoc, R, TemplateArgs, S);
 }