aaron.ballman added inline comments.
================
Comment at: clang/lib/Parse/ParseStmt.cpp:886-887
TopLevelCase = Case;
- else
+ else {
+ assert(DeepestParsedCaseStmt && "DeepestParsedCaseStmt cannot be
null");
Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, Case.get());
----------------
The `assert` doesn't add much value, this seems to be a false positive in the
analysis.
If we have no top-level case statement yet, then we get into the `if` branch on
line 884, but that then sets `DeepestParsedCaseStmt` on line 890, and that must
be non-null because we verified that `Case` is valid above on line 876. So by
the time we get into the `else` branch, `DeepestParsedCaseStmt` must be nonnull.
I don't think changes are needed here at all; WDYT?
================
Comment at: clang/lib/Sema/SemaExprObjC.cpp:2441
+ assert(receiverTypeInfo && "receiverTypeInfo cannot be null");
return BuildClassMessage(receiverTypeInfo, ReceiverType,
----------------
schittir wrote:
> Fznamznon wrote:
> > That seems to be a strange place before and after changes. With or without
> > change, when `ReceiverType.isNull()` is true we just end up passing
> > `nullptr` as `receiverTypeInfo ` to the `BuildClassMessage` which doesn't
> > seem to be checking its non-nullness before dereferencing it, even though
> > its description says that `receiverTypeInfo` can be null.
> > I guess it is fine to pass `nullptr` to a function whose description says
> > so, but the non-nullness check inside of it should be probably a bit more
> > obvious than it is right now.
> I see your point about passing `ReceiverTypeInfo` as nullptr to
> `BuildClassMessage` method - it seems ok to do that.
> Would it make sense to add an `assert(ReceiverTypeInfo);` inside the method
> as way of making the non-nullness check more obvious?
I think there's a different assertion needed here. The invariant that
`BuildClassMessage()` has is that the first argument (the `TypeSourceInfo *`
needs to be nonnull if the third argument (the `SourceLocation`) is invalid. So
I think this assert should be:
```
assert(((isSuperReceiver && Loc.isValid()) || receiverTypeInfo) &&
"Either the super receiver location needs to be valid or the receiver
needs valid type source information");
```
================
Comment at: clang/lib/Sema/SemaType.cpp:2711-2712
// Only support _BitInt elements with byte-sized power of 2 NumBits.
if (CurType->isBitIntType()) {
- unsigned NumBits = CurType->getAs<BitIntType>()->getNumBits();
+ unsigned NumBits = CurType->castAs<BitIntType>()->getNumBits();
if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
----------------
Slightly better fix so we don't do "isa" followed by "cast".
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153236/new/
https://reviews.llvm.org/D153236
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
