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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
