jtmott-intel marked an inline comment as done.
jtmott-intel added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:327
return true;
TheCall->setArg(2, Arg.get());
}
----------------
rjmccall wrote:
> jtmott-intel wrote:
> > rjmccall wrote:
> > > I know this is existing code, but this is a broken mess. Please change
> > > this to:
> > >
> > > ```
> > > ExprResult Arg =
> > > DefaultFunctionArrayLvalueConversion(TheCall->getArg(2));
> > > if (Arg.isInvalid()) return true;
> > > TheCall->setArg(2, Arg.get());
> > >
> > > QualType Ty = Arg.get()->getType();
> > > const auto *PtrTy = Ty->getAs<PointerType>();
> > > if (!PtrTy ||
> > > !PtrTy->getPointeeType()->isIntegerType() ||
> > > PtrTy->getPointeeType().isConstQualified()) {
> > > S.Diag(Arg.get()->getBeginLoc(),
> > > diag::err_overflow_builtin_must_be_ptr_int)
> > > << Ty << Arg.get()->getSourceRange();
> > > return true;
> > > }
> > > ```
> > >
> > > Test case would be something like (in ObjC):
> > >
> > > ```
> > > @interface HasPointer
> > > @property int *pointer;
> > > @end
> > >
> > > void test(HasPointer *hp) {
> > > __builtin_add_overflow(x, y, hp.pointer);
> > > }
> > > ```
> > >
> > > And the earlier block needs essentially the same change (but obviously
> > > checking for a different type). You can't check types before doing
> > > placeholder conversions, and once you do l-value conversions and a
> > > type-check, you really don't need the full parameter-initialization logic.
> > I've implemented this locally but I have a quick question about the test.
> > It passed even before I applied this code change. Is this test expected to
> > fail before this change and pass after? Or is it just to prove that the
> > feature (placeholder argument types?) works in general?
> Oh, we seem to handle placeholders specially, nevermind. Well, what I said
> about doing conversions before checking the type still holds, but the test
> case is just this:
>
> ```
> int z[1];
> __builtin_add_overflow(x, y, z);
> ```
>
> where we should decay the argument to a pointer instead of rejecting it.
Done. Latest updated patch includes these code changes.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81420/new/
https://reviews.llvm.org/D81420
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
