[PATCH] D85025: [RecoveryExpr]WIP: Support dependence in C-only codepath

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D85025#2201042 <https://reviews.llvm.org/D85025#2201042>, @rsmith wrote:

> Looks reasonable to me. I expect you'll find more places that need to learn how to handle dependent types in C, but this looks like a solid start.

Thanks! Yeah, we'd need to a detailed plan to roll this out (similar to the recovery-ast in C++) to catch possibly-missing cases.

I'll address your comments in split patches. I think the next plan is be to move forward the code reviews, Sam agreed to review those and I'll add you in the Subscribers.



================
Comment at: clang/lib/AST/Expr.cpp:3757-3758
     case NPC_ValueDependentIsNull:
-      if (isTypeDependent() || getType()->isIntegralType(Ctx))
+      if ((!containsErrors() && isTypeDependent()) ||
+          getType()->isIntegralType(Ctx))
         return NPCK_ZeroExpression;
----------------
rsmith wrote:
> This change appears to be redundant: we handled all `containsErrors()` cases before the `switch`.
oh, yeah, this is an oversight during the rebase.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:14245-14247
+ExprResult Sema::CreateDependentBinOp(SourceLocation OpLoc,
+                                      BinaryOperatorKind Opc, Expr *LHS,
+                                      Expr *RHS) {
----------------
rsmith wrote:
> This function seems dangerous: in C++, we need to perform unqualified lookups from the template definition context when creating a dependent binary operator, and it's only correct to use this if such lookup found nothing.
> 
> Perhaps you could add something to the name of the function to indicate that it should only be used when there are no unqualified lookup results?
good point, thanks! I added some comments about this method, and make it private to make it less mis-unused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85025