leonardchan added inline comments.
================
Comment at: clang/lib/Sema/SemaCast.cpp:2557-2558
return;
+ CheckNoDeref(Self, SrcExpr.get()->getType(), ResultType,
+ OpRange.getBegin());
}
----------------
rsmith wrote:
> 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?
No, you're right. I accidentally left this in.
================
Comment at: clang/lib/Sema/SemaCast.cpp:2982
+ CheckNoDeref(*this, Op.SrcExpr.get()->getType(), Op.ResultType, LPLoc);
+
----------------
rsmith wrote:
> As above, should we be checking this for a C-style cast? (It also looks like
> we're doing this check twice in C++.)
Also accidentally left this in.
================
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()) {
----------------
rsmith wrote:
> 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?
Hmm. I think for this particular location, `static_cast` contexts are all we
need to suppress it for to prevent duplicate warnings from showing.
We don't need to suppress for these since the attribute is allowed to pass
through these:
```
IC_CStyleCast
IC_FunctionalCast
```
We perform the check for implicit casts here since (AFAICT) only explicit casts
(C-style, functional, and C++ style) seem to be covered in `SemaCast`:
```
IC_Implicit
```
This would cover cases like `int *ptr = noderef_ptr;`.
I'm not sure about these ones, but none of the cases I have seem to fail if I
suppress only them:
```
IC_Normal
IC_ExplicitConvs
```
It could be though that there's a particular case I'm not covering in my tests.
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
