-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Don’t dive into null parts of AST. #18153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don’t dive into null parts of AST. #18153
Conversation
@swift-ci please smoke test |
@akyrtzi Did you want to look at this? Thanks! |
lib/AST/USRGeneration.cpp
Outdated
@@ -226,7 +226,7 @@ bool ide::printDeclUSR(const ValueDecl *D, raw_ostream &OS) { | |||
return printObjCUSR(D, OS); | |||
} | |||
|
|||
if (!D->hasInterfaceType()) | |||
if (!D->hasInterfaceType() || D->getInterfaceType()->is<ErrorType>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the reason the test is filing, could you verify whether this change is necessary, is it really fixing a crash in your original test case ?
Generally it would be ideal if you could add a regression test but I can understand if it is too difficult to reduce the original failing test case. |
@swift-ci please smoke test |
I agree a regression test would be great. If I can come up with one, I'll do another PR. |
…uces-no-output Don’t dive into null parts of AST. # Conflicts: # lib/IDE/SourceEntityWalker.cpp
When compiling code with certain errors, it is possible to get null pointers in the AST.
The compiler indexes even when there are errors, and the indexing code then tries to walk into the null pointers, causing a crash. As a result, the frontend crashes, and the developer does not get to see the errors. This PR adds a null check for this specific case. It does not solve the general problem.
Resolves rdar://42314665