sammccall added inline comments.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:6437
+ if (getLangOpts().CDependence &&
+ (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs)))
{
----------------
why is CDependence a condition and not an assertion inside the if?
(If it's for performance, then we're going to lose this performance when
flipping the flag, so I'd consider just flipping it now)
If you're using it as a short-hand for "is this plain C and also has recovery
enabled" then I'd avoid that - it's not clear that this is a language check.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:6444
+ "should only occur in error-recovery path.");
+ return CallExpr::Create(Context, Fn, ArgExprs, Context.DependentTy,
+ VK_RValue, RParenLoc);
----------------
Is this really the right place vs in BuildResolvedCallExpr?
================
Comment at: clang/lib/Sema/SemaExpr.cpp:6444
+ "should only occur in error-recovery path.");
+ return CallExpr::Create(Context, Fn, ArgExprs, Context.DependentTy,
+ VK_RValue, RParenLoc);
----------------
sammccall wrote:
> Is this really the right place vs in BuildResolvedCallExpr?
why DependentTy? shouldn't it be the return type of the function, if available?
================
Comment at: clang/test/AST/ast-dump-recovery.c:70
+ // in Sema::CheckPlaceholderExpr.
+ // CHECK-NOT: DeclRefExpr {{.*}} 'ext' 'int (int)'
+ ext(undef_var);
----------------
i'm not sure what the `int (int)` is about - can you remove it from the
assertion?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84304/new/
https://reviews.llvm.org/D84304
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
