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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

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.



================
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;
----------------
This change appears to be redundant: we handled all `containsErrors()` cases before the `switch`.


================
Comment at: clang/lib/Sema/SemaCast.cpp:2693
+      (DestType->isDependentType() || SrcExpr.get()->isTypeDependent() ||
+       SrcExpr.get()->isValueDependent())) {
+    assert((DestType->containsErrors() || SrcExpr.get()->containsErrors() ||
----------------
Do we care about value-dependence here? Very little of C cast semantics depends on the evaluated value of the expression -- I think this only matters for null pointer constants. If we care about the value-dependent cast to pointer case, maybe we should special-case that?

(It looks like the special-case handling for OpenCL event_t will also need a value-dependence check.)


================
Comment at: clang/lib/Sema/SemaExpr.cpp:14245-14247
+ExprResult Sema::CreateDependentBinOp(SourceLocation OpLoc,
+                                      BinaryOperatorKind Opc, Expr *LHS,
+                                      Expr *RHS) {
----------------
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?


================
Comment at: clang/test/Sema/error-dependence.c:10
+
+  // verify disgnostic "called object type '<dependent type>' is not a function
+  // or function pointer" is not emitted.
----------------



================
Comment at: clang/test/Sema/typo-correction.c:17
 int foobar;  // expected-note {{'foobar' declared here}}
-a = goobar ?: 4;  // expected-warning {{type specifier missing, defaults to 'int'}} \
+new_a = goobar ?: 4;  // expected-warning {{type specifier missing, defaults to 'int'}} \
                   // expected-error {{use of undeclared identifier 'goobar'; did you mean 'foobar'?}} \
----------------
I assume we're getting a redefinition error for `a` now. If so, can you test that uses of `a` after line 13 don't produce errors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85025