WIP/SwiftExpressionParser: preserve default argument thunks #1744
Conversation
|
@jimingham you understand the expression parser better than me. Can you please review this patch? It fixes default arguments in the REPL. |
There was a problem hiding this comment.
I'm not up on how swift does default arguments. It looks from your change like Swift emits little assistant functions that set the parameters to the default values. If that's right, why doesn't swift::performSILGeneration emit them automatically? Why do you have to go back and force their emission by hand?
| @@ -788,6 +788,44 @@ void SwiftASTManipulator::FindNonVariableDeclarations( | |||
| } | |||
| } | |||
|
|
|||
| void SwiftASTManipulator::EnumerateDefaultValuedArguments(llvm::SmallVectorImpl<std::pair<swift::FuncDecl *, std::vector<int>>> &FDAI) { | |||
| class DefaultArgumentThunkFinder : public swift::ASTWalker { | |||
| swift::SourceFile *SF; | |||
There was a problem hiding this comment.
Can you use variable names more in keeping with lldb's conventions. Particularly here, you have FD DAI, AI, DAI... Playing the "guess the acronym" game makes the code much harder to read.
|
Also, if this fixes something there should be tests for what it fixes. |
| for (int AI : DAI) { | ||
| auto DAG = swift::SILDeclRef::getDefaultArgGenerator(FD, AI); | ||
| swift::SILFunction *F = sil_module->lookUpFunction(DAG); | ||
| F->dump(); |
There was a problem hiding this comment.
I don't see how this has any effect (besides spewing garbage to stderr). What's actually going on here?
There was a problem hiding this comment.
I disagree with the spewing garbage bit at least - it was very useful to me when I was trying to determine if I was catching everything and the correct thing :-). Yes, this is absolutely debugging cruft that is not meant to be merged.
|
@jimingham - it doesn't because they may not actually be needed - in the case that the value is provided, the call is not needed. I never finished this work, and there is more that needs to be done here, if you want to pick it up, that would be great. I've been busy with the breakage in libdispatch, Foundation, llbuild (across android, Windows) and trying to get s-p-m bootstrap builds CI'ed for Windows. |
No description provided.