[AST] Avoid tripping assertion in IsNonUserModuleRequest
#77636
Conversation
Not entirely sure when this can happen, but we have been seeing reports of crashes in `IsNonUserModuleRequest` due to the runtime assertion in `getFiles`. Use the equivalent const method that doesn't assert - it's not an issue if we just return `false` here. Resolves rdar://137769081.
|
@swift-ci please test |
|
Edit: Hmm nevermind, I see that |
| ArrayRef<const FileUnit *> modFiles = mod->getFiles(); | ||
| if (modFiles.empty()) |
There was a problem hiding this comment.
Worth adding a comment given the subtlety of the fix? This all stems from the fact that I missed that there was another overload of getFiles when I originally added the assert 😅
There was a problem hiding this comment.
Do you remember what you added it for? Honestly I kind of hate this and am tempted to just remove the assert instead.
There was a problem hiding this comment.
IIRC it was added to help enforce that getFiles isn't called before the files are added to the module, the ultimate goal is to replace addFile with a setFiles method that can only be called once (or giving ModuleDecl a lambda constructor with which to populate the files)
There was a problem hiding this comment.
Let's try this #77666
Yeah, we're definitely asserting here. The particular crashes I see are when retrieving cursor info, so it's likely this is due to invalid code. My preference would be fixing everywhere to never have a module with no files, but it's unclear to me where that's happening 😅 |
Not entirely sure when this can happen, but we have been seeing reports of crashes in
IsNonUserModuleRequestdue to the runtime assertion ingetFiles. Use the equivalent const method that doesn't assert - it's not an issue if we just returnfalsehere.Resolves rdar://137769081.