Allowing explicit @escaping for optional closures in function parameters

GLanza · September 4, 2019, 2:24pm

Any other suggestions on how to change the proposal ?
Which would be the next step to get it implemented? Should I partner with someone from the core team? It doesn't seem to be something that could be implemented by a beginner in swift code base.

John_McCall · September 4, 2019, 7:46pm

You don't want someone from the "core team", which is the project design/leadership group; you want someone with implementation expertise. If you're willing to do the implementation work yourself, @Andrew_Trick knows the mandatory-escape-analysis code and told me he was willing to guide you through what you'd need to do.

GLanza · September 4, 2019, 9:20pm

I'm absolutely more than happy to do the work myself under the supervision of @Andrew_Trick , if it's ok for him.

Thank you so much @John_McCall and @Andrew_Trick

Andrew_Trick · September 4, 2019, 10:29pm

I do think this is a serious usability flaw that should be fixed. Unfortunately implementing the diagnostic is involved. Let me know if this makes sense @Slava_Pestov @xedin ...

My intuition is that the existing diagnostics can be extended to handle optional function types. Most of those diagnostics are in the type checker's constraint solver (see MarkExplicitlyEscaping). Currently, whenever the type system unwraps an optional function type, it assumes an escaping type. Instead, we need to give the optional a noescape type (see TVO_CanBindToNoEscape) and infer a noescape function type when unwraps the optional type. Then, the actual diagnostics within MarkExplicitlyEscaping need to be generalized for non-function types.

A secondary round of diagnostics in SIL ensures that nonescaping functions don't aren't captured by escaping closures. This is the part I'm familiar with. It's the DiagnoseInvalidEscapingCaptures pass.

It checks that @noescape types aren't captured by an escaping closure:

github.com

apple/swift/blob/8d1fabd859763c2d9488a98f8c5009fa788d699a/lib/SILOptimizer/Mandatory/DiagnoseInvalidEscapingCaptures.cpp#L345


      
                diagnose(Context, PAI->getLoc(), diag::escaping_inout_capture,
                          param->getName());
                diagnose(Context, param->getLoc(), diag::inout_param_defined_here,
                          param->getName());
              }
            }
          
          
  diagnoseCaptureLoc(Context, DC, PAI, inoutCapture);
          }
          
          
// Finally, diagnose captures of values with noescape type.
          for (auto noEscapeCapture : noEscapeCaptures) {
            if (auto *param = getParamDeclFromOperand(noEscapeCapture->get())) {
              diagnose(Context, PAI->getLoc(), diag::escaping_noescape_param_capture,
                       param->getName());
              diagnose(Context, param->getLoc(), diag::noescape_param_defined_here,
                       param->getName());
            } else {
              diagnose(Context, PAI->getLoc(), diag::escaping_noescape_var_capture);
            }

And that @noescape function parameters aren't passed into other @noescape functions

github.com

apple/swift/blob/8d1fabd859763c2d9488a98f8c5009fa788d699a/lib/SILOptimizer/Mandatory/DiagnoseInvalidEscapingCaptures.cpp#L417


      
          while (!args.empty()) {
            auto pair = args.pop_back_val();
            auto arg = pair.first;
            bool capture = pair.second;
          
          
  if (auto *CI = dyn_cast<ConversionInst>(arg)) {
              arglistInsert(CI->getConverted(), /*capture=*/false);
              continue;
            }
          
          
  // If one of our call arguments is a noescape parameter, diagnose the
            // violation.
            if (auto *param = isNoEscapeParam(arg)) {
              diagnose(Context, site.getLoc(), diag::err_noescape_param_call,
                       param->getName(), capture);
              return;
            }
          
          
  // If one of our call arguments is a closure, recursively visit all of
            // the closure's captures.
            if (auto *PAI = dyn_cast<PartialApplyInst>(arg)) {

Both of these would also need to be extended for optionals. checkNoEscapePartialApplyUse also needs to be fixed to recognized that nonescaping closures can be stored a nonescaping optional. Since optionals don't have a special representation in SIL, it needs to look for an 'enum' instruction. These extensions to the pass should not be too hard as long as optional's AST type can be marked "noescape". See TypeBase::isNoEscape().

Slava_Pestov · September 6, 2019, 8:50pm

One thing to note is that SILGen and the SIL optimizer passes already have some minimal support for optionals of no-escape type, because even though you cannot write such a type in source, it comes up in the signatures of imported Objective-C methods, and therefore, bridging thunks emitted by SIL.

xedin · September 7, 2019, 6:27am

@Andrew_Trick Yes, what you have mentioned is exactly what needs to be done. We'd just need to special case optional function type case where Wrapped generic parameter should be opened into a type variable with TVO_CanBindToNoEscape flag, or (most likely) add a new TVO_CanBindToNoEscapeFunction flag to make it more distinct, and adjust diagnostics related to MarkExplicitlyEscaping accordingly. It seems like this should be feasible for beginners. @GLanza if you need any help with implementation please let me know.