[AST] Make Expr::setDependence protected and remove add/removeDependence. NFC

[sammccall]

Summary

The expected pattern is for subclasses to initialize through
computeDependence, which needs only setDependence.
The few places that still use addDependence can be simulated with get+set.

[hokein]

clang/include/clang/AST/Expr.h

2772

the method name seems too concrete to me, I'd prefer to drop ForPostponedNameLookup, though there is only one usage in Sema.

[sammccall]

clang/include/clang/AST/Expr.h

2772

So the idea here is that we're breaking encapsulation for a special purpose, and this method shouldn't be generally used to manipulate dependence.
If we do want to add another usage (in sema or elsewhere) for another purpose, it makes sense to have to modify Expr (by renaming the method or adding another one), just as if this function was private.

[sammccall]

Add comment on markDependentForPostponedNameLookup

clang/include/clang/AST/Expr.h

Expr(StmtClass SC, QualType T, ExprValueKind VK, ExprObjectKind OK)
     ExprBits.ObjectKind = OK;
     assert(ExprBits.ObjectKind == OK && "truncated kind");
     setType(T);
   }
 
   /// Construct an empty expression.
   explicit Expr(StmtClass SC, EmptyShell) : ValueStmt(SC) { }
 
+  /// Each concrete expr subclass is expected to compute its dependence and call
+  /// this in the constructor.
+  void setDependence(ExprDependence Deps) {
+    ExprBits.Dependent = static_cast<unsigned>(Deps);
+  }
+  friend class ASTImporter; // Sets dependence dircetly.
+  friend class ASTStmtReader; // Sets dependence dircetly.
+
 public:
   QualType getType() const { return TR; }
   void setType(QualType t) {
     // In C++, the type of an expression is always adjusted so that it
     // will not have reference type (C++ [expr]p6). Use
     // QualType::getNonReferenceType() to retrieve the non-reference
     // type. Additionally, inspect Expr::isLvalue to determine whether
     // an expression that is adjusted in this manner should be
     // considered an lvalue.
     assert((t.isNull() || !t->isReferenceType()) &&
            "Expressions can't have reference type");
 
     TR = t;
   }
 
   ExprDependence getDependence() const {
     return static_cast<ExprDependence>(ExprBits.Dependent);
   }
 
-  /// Each concrete expr subclass is expected to compute its dependence and call
-  /// this in the constructor.
-  void setDependence(ExprDependence Deps) {
-    ExprBits.Dependent = static_cast<unsigned>(Deps);
-  }
-  void addDependence(ExprDependence Deps) {
-    ExprBits.Dependent |= static_cast<unsigned>(Deps);
-  }
-  void removeDependence(ExprDependence Deps) {
-    ExprBits.Dependent &= ~static_cast<unsigned>(Deps);
-  }
-
   /// isValueDependent - Determines whether this expression is
   /// value-dependent (C++ [temp.dep.constexpr]). For example, the
   /// array bound of "Chars" in the following example is
   /// value-dependent.
   /// @code
   /// template<int Size, char (&Chars)[Size]> struct meta_string;
   /// @endcode
   bool isValueDependent() const {
public:
 
   SourceLocation getBeginLoc() const LLVM_READONLY;
   SourceLocation getEndLoc() const LLVM_READONLY;
 
   /// Return true if this is a call to __assume() or __builtin_assume() with
   /// a non-value-dependent constant parameter evaluating as false.
   bool isBuiltinAssumeFalse(const ASTContext &Ctx) const;
 
+  /// Used by Sema to implement MSVC-compatible delayed name lookup.
+  /// (Usually Exprs themselves should set dependence).
+  void markDependentForPostponedNameLookup() {
+    setDependence(getDependence() | ExprDependence::TypeValueInstantiation);
+  }
+
   bool isCallToStdMove() const {
     const FunctionDecl *FD = getDirectCallee();
     return getNumArgs() == 1 && FD && FD->isInStdNamespace() &&
            FD->getIdentifier() && FD->getIdentifier()->isStr("move");
   }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() >= firstCallExprConstant &&
Expr *getInit(unsigned Init) {
     return cast_or_null<Expr>(InitExprs[Init]);
   }
 
   void setInit(unsigned Init, Expr *expr) {
     assert(Init < getNumInits() && "Initializer access out of range!");
     InitExprs[Init] = expr;
 
     if (expr)
-      addDependence(expr->getDependence());
+      setDependence(getDependence() | expr->getDependence());
   }
 
   /// Reserve space for some number of initializers.
   void reserveInits(const ASTContext &C, unsigned NumInits);
 
   /// Specify the number of initializers
   ///
   /// If there are more than @p NumInits initializers, the remaining

clang/lib/AST/ComputeDependence.cpp

ExprDependence clang::computeDependence(OffsetOfExpr *E) {
   auto D = turnTypeToValueDependence(
       toExprDependence(E->getTypeSourceInfo()->getType()->getDependence()));
   for (unsigned I = 0, N = E->getNumExpressions(); I < N; ++I)
     D |= turnTypeToValueDependence(E->getIndexExpr(I)->getDependence());
   return D;
 }
 
 ExprDependence clang::computeDependence(MemberExpr *E) {
-  return E->getBase()->getDependence();
+  auto *MemberDecl = E->getMemberDecl();
+  auto D = E->getBase()->getDependence();
+  if (FieldDecl *FD = dyn_cast<FieldDecl>(MemberDecl)) {
+    DeclContext *DC = MemberDecl->getDeclContext();
+    // dyn_cast_or_null is used to handle objC variables which do not
+    // have a declaration context.
+    CXXRecordDecl *RD = dyn_cast_or_null<CXXRecordDecl>(DC);
+    if (RD && RD->isDependentContext() && RD->isCurrentInstantiation(DC)) {
+      if (!E->getType()->isDependentType())
+        D &= ~ExprDependence::Type;
+    }
+
+    // Bitfield with value-dependent width is type-dependent.
+    if (FD && FD->isBitField() && FD->getBitWidth()->isValueDependent()) {
+      D |= ExprDependence::Type;
+    }
+  }
+  // FIXME: move remaining dependence computation from MemberExpr::Create()
+  return D;
 }
 
 ExprDependence clang::computeDependence(InitListExpr *E) {
   auto D = ExprDependence::None;
   for (auto *A : E->inits())
     D |= A->getDependence();
   return D;
 }

clang/lib/AST/Expr.cpp

std::size_t Size =
                        TemplateArgumentLoc>(
           HasQualOrFound ? 1 : 0, HasTemplateKWAndArgsInfo ? 1 : 0,
           TemplateArgs ? TemplateArgs->size() : 0);
 
   void *Mem = C.Allocate(Size, alignof(MemberExpr));
   MemberExpr *E = new (Mem) MemberExpr(Base, IsArrow, OperatorLoc, MemberDecl,
                                        NameInfo, T, VK, OK, NOUR);
 
-  if (isa<FieldDecl>(MemberDecl)) {
-    DeclContext *DC = MemberDecl->getDeclContext();
-    // dyn_cast_or_null is used to handle objC variables which do not
-    // have a declaration context.
-    CXXRecordDecl *RD = dyn_cast_or_null<CXXRecordDecl>(DC);
-    if (RD && RD->isDependentContext() && RD->isCurrentInstantiation(DC)) {
-      if (E->isTypeDependent() && !T->isDependentType())
-        E->removeDependence(ExprDependence::Type);
-    }
-    // Bitfield with value-dependent width is type-dependent.
-    FieldDecl *FD = dyn_cast<FieldDecl>(MemberDecl);
-    if (FD && FD->isBitField() && FD->getBitWidth()->isValueDependent())
-      E->addDependence(ExprDependence::Type);
-  }
-
+  // FIXME: remove remaining dependence computation to computeDependence().
+  auto Deps = E->getDependence();
   if (HasQualOrFound) {
     // FIXME: Wrong. We should be looking at the member declaration we found.
     if (QualifierLoc && QualifierLoc.getNestedNameSpecifier()->isDependent())
-      E->addDependence(ExprDependence::TypeValueInstantiation);
+      Deps |= ExprDependence::TypeValueInstantiation;
     else if (QualifierLoc &&
              QualifierLoc.getNestedNameSpecifier()->isInstantiationDependent())
-      E->addDependence(ExprDependence::Instantiation);
+      Deps |= ExprDependence::Instantiation;
 
     E->MemberExprBits.HasQualifierOrFoundDecl = true;
 
     MemberExprNameQualifier *NQ =
         E->getTrailingObjects<MemberExprNameQualifier>();
     NQ->QualifierLoc = QualifierLoc;
     NQ->FoundDecl = FoundDecl;
   }
 
   E->MemberExprBits.HasTemplateKWAndArgsInfo =
       TemplateArgs || TemplateKWLoc.isValid();
 
   if (TemplateArgs) {
-    auto Deps = TemplateArgumentDependence::None;
+    auto TemplateArgDeps = TemplateArgumentDependence::None;
     E->getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
         TemplateKWLoc, *TemplateArgs,
-        E->getTrailingObjects<TemplateArgumentLoc>(), Deps);
-    if (Deps & TemplateArgumentDependence::Instantiation)
-      E->addDependence(ExprDependence::Instantiation);
+        E->getTrailingObjects<TemplateArgumentLoc>(), TemplateArgDeps);
+    if (TemplateArgDeps & TemplateArgumentDependence::Instantiation)
+      Deps |= ExprDependence::Instantiation;
   } else if (TemplateKWLoc.isValid()) {
     E->getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
         TemplateKWLoc);
   }
+  E->setDependence(Deps);
 
   return E;
 }
 
 MemberExpr *MemberExpr::CreateEmpty(const ASTContext &Context,
                                     bool HasQualifier, bool HasFoundDecl,
                                     bool HasTemplateKWAndArgsInfo,
                                     unsigned NumTemplateArgs) {

clang/lib/AST/ExprConcepts.cpp

RequiresExpr::RequiresExpr(ASTContext &C, SourceLocation RequiresKWLoc,
   }
   std::copy(LocalParameters.begin(), LocalParameters.end(),
             getTrailingObjects<ParmVarDecl *>());
   std::copy(Requirements.begin(), Requirements.end(),
             getTrailingObjects<concepts::Requirement *>());
   RequiresExprBits.IsSatisfied |= Dependent;
   // FIXME: move the computing dependency logic to ComputeDependence.h
   if (ContainsUnexpandedParameterPack)
-    addDependence(ExprDependence::UnexpandedPack);
+    setDependence(getDependence() | ExprDependence::UnexpandedPack);
   // FIXME: this is incorrect for cases where we have a non-dependent
   // requirement, but its parameters are instantiation-dependent. RequiresExpr
   // should be instantiation-dependent if it has instantiation-dependent
   // parameters.
   if (Dependent)
-    addDependence(ExprDependence::ValueInstantiation);
+    setDependence(getDependence() | ExprDependence::ValueInstantiation);
 }
 
 RequiresExpr::RequiresExpr(ASTContext &C, EmptyShell Empty,
                            unsigned NumLocalParameters,
                            unsigned NumRequirements)
   : Expr(RequiresExprClass, Empty), NumLocalParameters(NumLocalParameters),
     NumRequirements(NumRequirements) { }
 

clang/lib/Sema/SemaOverload.cpp

if (CandidateSet->empty() ||
         CandidateSet->BestViableFunction(*this, Fn->getBeginLoc(), Best) ==
             OR_No_Viable_Function) {
       // In Microsoft mode, if we are inside a template class member function
       // then create a type dependent CallExpr. The goal is to postpone name
       // lookup to instantiation time to be able to search into type dependent
       // base classes.
       CallExpr *CE = CallExpr::Create(Context, Fn, Args, Context.DependentTy,
                                       VK_RValue, RParenLoc);
-      CE->addDependence(ExprDependence::TypeValueInstantiation);
+      CE->markDependentForPostponedNameLookup();
       *Result = CE;
       return true;
     }
   }
 
   if (CandidateSet->empty())
     return false;