Add check against NULL before dereferencing pointer #62724
Conversation
lib/IDE/Formatting.cpp
Outdated
| AbstractClosureExpr *CE = CL->getClosureBody(); | ||
| BraceStmt *BS = CE->getBody(); | ||
| BraceStmt *BS = CE ? CE->getBody() : NULL; |
There was a problem hiding this comment.
None of these can ever be a null pointer, but the invariants are neglected. Can we assert this for the closure upon CaptureListExpr construction, and remove the always true hasBody() check from AbstractClosureExpr::getBody? This also seems to be the only remaining use of hasBody(), so we may as well just delete it.
There was a problem hiding this comment.
lib/SIL/IR/SILProfiler.cpp still uses hasBody() in assert. But if hasBody() always returns true this assert could also be removed.
There was a problem hiding this comment.
@AnthonyLatsis, are you sure I should remove hasBody() completely?
AnthonyLatsis
added
code health
compiler
lib/IDE/Formatting.cpp
Outdated
| @@ -2485,8 +2485,9 @@ class FormatWalker : public ASTWalker { | |||
| getIndentContextFrom(CaptureListExpr *CL, | |||
| SourceLoc ContextLoc = SourceLoc()) { | |||
| AbstractClosureExpr *CE = CL->getClosureBody(); | |||
| assert(CE); | |||
There was a problem hiding this comment.
Not here, in the CaptureListExpr constructor; sorry if I wasn’t clear.
There was a problem hiding this comment.
Sorry, I get lost. By constructor do you mean CaptureListExpr::create? If so what should I assert there? assert(mem);?
There was a problem hiding this comment.
By constructor do you mean
CaptureListExpr::create?
Yes, but preferably the actual constructor, CaptureListExpr::CaptureListExpr. It’s defined in the header.
If so what should I assert there?
assert(closureBody).
I don't remember exactly why I added it. I think it was to mirror another API? Maybe the IIRC, the calls underlying |
|
@AnthonyLatsis's suggestions make sense to me. |
Good point. |
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
RxSwift failure is unrelated, merging. |
Condition following fix suggests pointer is expected sometimes to be NULL.