rjmccall added a comment.
Thanks. Just a few small requests to re-use more of the existing logic here.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:7659
+ << 0 << 2 << TheCall->getNumArgs() << Callee->getSourceRange();
+ }
+
----------------
It looks like this should be `if (NumArgs < 2)`.
We actually have helper functions in this file for doing this kind of arg-count
checking, but it looks like we don't have one that takes a range. Could you
just generalize `checkArgCount` so that it takes a min and max arg count? You
can make the existing function call your generalization.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:7671
+ AllArgs, CallType))
+ return true;
+
----------------
You can just pull the argument expressions out of the `CallExpr`; you don't
need to call `GatherArgumentsForCall`.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:7684
+ if (FirstArgResult.isInvalid())
+ return true;
+
----------------
There is in fact a `DefaultFunctionArrayLvalueConversion` method you can use to
do all of this at once, and you don't need to explicitly check for a function
or array type first.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:7690
+ // cast instruction which cast type from user-written-type to VoidPtr in
+ // CodeGen.
+ AllArgs[0] = FirstArgResult.get();
----------------
This comment doesn't mean anything in the context of the current
implementation. If you want to keep something like it (you really don't need
to), you could have a comment at the top explaining that we use custom
type-checking because it accepts an arbitrary pointer type as the first
argument.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:7695
+ TheCall->setArg(i, AllArgs[i]);
+ TheCall->computeDependence();
+
----------------
This is really just `TheCall->setArg(i, FirstArgResult.get())`.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:7701
// We can't check the value of a dependent argument.
- if (!Arg->isTypeDependent() && !Arg->isValueDependent()) {
+ if (!SecondArg->isTypeDependent() && !SecondArg->isValueDependent()) {
llvm::APSInt Result;
----------------
Type-dependence implies value-dependence, so you can just check for the latter.
You should call `convertArgumentToType` with `size_t` in this block, just in
case the argument is something like an l-value reference to a `const` integer
variable.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:7719
+ Context, Context.getSizeType(), false);
+ ThirdArg = PerformCopyInitialization(Entity, SourceLocation(), ThirdArg);
+ if (ThirdArg.isInvalid())
----------------
You can use `convertArgumentToType` here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131979/new/
https://reviews.llvm.org/D131979
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
