Skip to content

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

Merged

Conversation

davidungar
Copy link
Contributor

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

@davidungar
Copy link
Contributor Author

@swift-ci please smoke test

@davidungar davidungar requested a review from akyrtzi July 23, 2018 20:52
@davidungar
Copy link
Contributor Author

@akyrtzi Did you want to look at this? Thanks!

@@ -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>())
Copy link
Contributor

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 ?

@akyrtzi
Copy link
Contributor

akyrtzi commented Jul 23, 2018

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.

@davidungar
Copy link
Contributor Author

@swift-ci please smoke test

@davidungar
Copy link
Contributor Author

I agree a regression test would be great. If I can come up with one, I'll do another PR.

@davidungar davidungar merged commit 1d68b11 into swiftlang:master Jul 24, 2018
akyrtzi added a commit that referenced this pull request Jul 27, 2018
davidungar pushed a commit to davidungar/swift that referenced this pull request Jul 30, 2018
…uces-no-output

Don’t dive into null parts of AST.
# Conflicts:
#	lib/IDE/SourceEntityWalker.cpp
nathawes pushed a commit to nathawes/swift that referenced this pull request Oct 8, 2018
@davidungar davidungar deleted the rdar-42314665-produces-no-output branch September 23, 2019 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants