[PATCH] D73638: [AST] Move dependence computations into a separate file

[Haojian Wu via Phabricator via cfe-commits]

hokein commandeered this revision.
hokein added a reviewer: ilya-biryukov.
hokein added a comment.

This patch contains too many changes, most of them are just NFC, it likely takes a long time to do a full review.

I actually did an review for the original patch. I have highlighted places (see me comments) that I'm uncertain and need another look, other places are NFC.



================
Comment at: clang/include/clang/AST/Expr.h:4081
-    : Expr(ConvertVectorExprClass, DstType, VK, OK,
-           DstType->isDependentType(),
-           DstType->isDependentType() || SrcExpr->isValueDependent(),
----------------
!  behavior change change, now the `ConvertVectorExpr::TypeDependent = DstType->isDependentType() | SrcExpr->isDependentType()`.


================
Comment at: clang/include/clang/AST/Expr.h:4139
-             bool TypeDependent, bool ValueDependent)
-    : Expr(ChooseExprClass, t, VK, OK, TypeDependent, ValueDependent,
-           (cond->isInstantiationDependent() ||
----------------
! this needs careful review, the ChooseExpr bits are from different sources:

- constructor here
- [`clang/lib/Sema/SemaPseudoObject.cpp`](https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/SemaPseudoObject.cpp#L170)
- [`clang/lib/AST/ASTImporter.cpp`](https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ASTImporter.cpp#L6347)
- [`clang/lib/Sema/SemaExpr.cpp`](https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/SemaExpr.cpp#L14207)

 


================
Comment at: clang/include/clang/AST/Expr.h:4250
             SourceLocation RPLoc, QualType t, bool IsMS)
-      : Expr(VAArgExprClass, t, VK_RValue, OK_Ordinary, t->isDependentType(),
-             false, (TInfo->getType()->isInstantiationDependentType() ||
----------------
! the implementation in the original patch doesn't seem to correct to me, I rewrote it, need another review..


================
Comment at: clang/include/clang/AST/Expr.h:5031
-             CommonInit->isValueDependent() || ElementInit->isValueDependent(),
-             T->isInstantiationDependentType(),
-             CommonInit->containsUnexpandedParameterPack() ||
----------------
! behavior change here, now `ArrayInitLoopExpr::Instantiation =  T->isInstantiationDependentType() |  CommonInit->isInstantiationDependentType() |  ElementInit->isInstantiationDependentType()`.


================
Comment at: clang/include/clang/AST/Expr.h:5650
-    : Expr(AsTypeExprClass, DstType, VK, OK,
-           DstType->isDependentType(),
-           DstType->isDependentType() || SrcExpr->isValueDependent(),
----------------
! behavior change here, now `Expr::TypeDependent = DstType->isDependentType() | SrcExpr->isTypeDependent()`.


================
Comment at: clang/include/clang/AST/Expr.h:1537
     CharacterLiteralBits.Kind = kind;
+    setDependence(ExprDependence::None);
   }
----------------
this is not needed indeed as the default value is 0, but I'd keep it here to make it more explicit.


================
Comment at: clang/include/clang/AST/ExprCXX.h:2742
-      : Expr(ArrayTypeTraitExprClass, ty, VK_RValue, OK_Ordinary,
-             false, queried->getType()->isDependentType(),
-             (queried->getType()->isInstantiationDependentType() ||
----------------
! behavior change here, now we the `ValueDependent`, `UnexpandedPack` takes `dimension` into account as well.


================
Comment at: clang/lib/AST/ExprCXX.cpp:444
-          SC, Context.OverloadTy, VK_LValue, OK_Ordinary, KnownDependent,
-          KnownDependent,
-          (KnownInstantiationDependent || NameInfo.isInstantiationDependent() ||
----------------
! this change is not trivial, need an review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73638/new/

https://reviews.llvm.org/D73638