Don't create a Module SwiftASTContext when the stdlib is missing #1391
Conversation
A Module SwiftASTContext that cannot access the stdlib module will crash when performing simple actions, like looking up a type. This patch tries to make the Target/Module overrides of CreateInstance look more like each other, w.r.t error handling in particular. rdar://57695158, rdar://64828733
|
@swift-ci test |
|
|
||
| ArchSpec arch = module.GetArchitecture(); | ||
| if (!arch.IsValid()) { | ||
| logError("invalid module architecture"); | ||
| return TypeSystemSP(); |
There was a problem hiding this comment.
I usually write the default return value as return {}, since the concrete type doesn't matter that much to the reader.
There was a problem hiding this comment.
Really just stylistic preference :-)
| if (!objfile) { | ||
| logError("no object file for module"); | ||
| return TypeSystemSP(); | ||
| } |
There was a problem hiding this comment.
do we want similar changes in CreateInstance(..., Target &) (the one that creates the expression context)?
There was a problem hiding this comment.
I looked through the other override and started re-arranging it, but decided to leave those changes out of this patch as they're entirely cosmetic. (I didn't find any missing checks.)
| @@ -1654,7 +1676,7 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(lldb::LanguageType language, | |||
| std::string target_triple; | |||
|
|
|||
| if (sym_file) { | |||
| bool got_serialized_options; | |||
| bool got_serialized_options = false; | |||
There was a problem hiding this comment.
This one is not so bad, I don't believe got_serialized_options is actually used, this is just some future-proofing. I can leave it out if you prefer.
A Module SwiftASTContext that cannot access the stdlib module will crash
when performing simple actions, like looking up a type.
This patch tries to make the Target/Module overrides of CreateInstance
look more like each other, w.r.t error handling in particular.
rdar://57695158, rdar://64828733