rsmith added inline comments.
================
Comment at: clang/lib/Sema/SemaCast.cpp:734
void CastOperation::CheckDynamicCast() {
+ CheckNoDerefRAII noderef_check(*this);
+
----------------
Please use UpperCamelCase for local variables.
================
Comment at: clang/lib/Sema/SemaCast.cpp:2557-2558
return;
+ CheckNoDeref(Self, SrcExpr.get()->getType(), ResultType,
+ OpRange.getBegin());
}
----------------
Should we be checking this for a C-style cast? You previously said you were
going to turn off the checks for C-style casts. Did you change your mind on
that?
================
Comment at: clang/lib/Sema/SemaCast.cpp:2982
+ CheckNoDeref(*this, Op.SrcExpr.get()->getType(), Op.ResultType, LPLoc);
+
----------------
As above, should we be checking this for a C-style cast? (It also looks like
we're doing this check twice in C++.)
================
Comment at: clang/lib/Sema/SemaInit.cpp:8182-8184
+ // Do not check static casts here because they are checked earlier
+ // in Sema::ActOnCXXNamedCast()
+ if (!Kind.isStaticCast()) {
----------------
leonardchan wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > Are there any `static_cast` cases that don't get here? I'm also a little
> > > surprised that `static_cast` would get here but the `static_cast`
> > > interpretation of a C-style or functional cast would not.
> > I'll double check on this. The reason I added this here was because the
> > cast in `static_cast_from_same_ptr_type()` triggered 2 of the
> > `warn_noderef_to_dereferenceable_pointer` warnings and one of them was
> > triggered here.
> Yeah it seems the `int *a = static_cast<int *>(x);` case in
> `cast_from_void_ptr()` is an example of a static_cast that doesn't go through
> here.
I see, this is only reached for the part of `static_cast` that considers
implicit conversion sequences. OK.
I expect there's similar overlap for all the other kinds of cast too. Should we
be suppressing this check for *all* casts, given that we perform the check in
`SemaCast` instead?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
