std::function -> llvm::function_ref for some non-escaping params.

[huonw]

std::function may need to allocate, which is totally unnecessary for non-escaping functions. llvm::function_ref covers this perfectly.

[huonw]

@swift-ci please test

[huonw]

@swift-ci Please ASAN test

[huonw]

@swift-ci Please smoke test compiler performance

[swift-ci]

Build comment file:

Summary for master smoketest

Unexpected test results, excluded stats for ReactiveCocoa

No regressions above thresholds

• Debug
  • debug brief
  • debug detailed
• Release
  • release brief
  • release detailed

Debug

debug brief

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (2)

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	41,026,876	41,027,938	1,062	0.0%
time.swift-driver.wall	68.1s	68.1s	67.0ms	0.1%

debug detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (23)

name	old	new	delta	delta_pct
AST.NumImportedExternalDefinitions	91,038	91,038	0	0.0%
AST.NumLoadedModules	12,956	12,956	0	0.0%
AST.NumTotalClangImportedEntities	259,146	259,146	0	0.0%
AST.NumUsedConformances	8,768	8,768	0	0.0%
IRModule.NumIRBasicBlocks	126,016	126,016	0	0.0%
IRModule.NumIRFunctions	74,367	74,367	0	0.0%
IRModule.NumIRGlobals	68,174	68,174	0	0.0%
IRModule.NumIRInsts	1,404,891	1,404,891	0	0.0%
IRModule.NumIRValueSymbols	127,379	127,379	0	0.0%
LLVM.NumLLVMBytesOutput	41,026,876	41,027,938	1,062	0.0%
SILModule.NumSILGenFunctions	43,427	43,427	0	0.0%
SILModule.NumSILOptFunctions	47,002	47,002	0	0.0%
Sema.NumConformancesDeserialized	334,008	334,008	0	0.0%
Sema.NumConstraintScopes	904,217	904,217	0	0.0%
Sema.NumDeclsDeserialized	2,239,613	2,239,613	0	0.0%
Sema.NumDeclsValidated	190,411	190,411	0	0.0%
Sema.NumFunctionsTypechecked	52,321	52,321	0	0.0%
Sema.NumGenericSignatureBuilders	61,112	61,112	0	0.0%
Sema.NumLazyGenericEnvironments	453,238	453,238	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	40,395	40,395	0	0.0%
Sema.NumLazyIterableDeclContexts	342,807	342,807	0	0.0%
Sema.NumTypesDeserialized	2,358,168	2,358,168	0	0.0%
Sema.NumTypesValidated	211,800	211,800	0	0.0%

Release

release brief

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (2)

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	30,638,224	30,638,224	0	0.0%
time.swift-driver.wall	165.0s	164.8s	-250.7ms	-0.15%

release detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (23)

name	old	new	delta	delta_pct
AST.NumImportedExternalDefinitions	8,764	8,764	0	0.0%
AST.NumLoadedModules	406	406	0	0.0%
AST.NumTotalClangImportedEntities	25,382	25,382	0	0.0%
AST.NumUsedConformances	8,774	8,774	0	0.0%
IRModule.NumIRBasicBlocks	176,308	176,308	0	0.0%
IRModule.NumIRFunctions	57,494	57,494	0	0.0%
IRModule.NumIRGlobals	55,469	55,469	0	0.0%
IRModule.NumIRInsts	1,399,339	1,399,339	0	0.0%
IRModule.NumIRValueSymbols	101,633	101,633	0	0.0%
LLVM.NumLLVMBytesOutput	30,638,224	30,638,224	0	0.0%
SILModule.NumSILGenFunctions	22,052	22,052	0	0.0%
SILModule.NumSILOptFunctions	38,777	38,777	0	0.0%
Sema.NumConformancesDeserialized	153,462	153,462	0	0.0%
Sema.NumConstraintScopes	798,881	798,881	0	0.0%
Sema.NumDeclsDeserialized	221,408	221,408	0	0.0%
Sema.NumDeclsValidated	59,864	59,864	0	0.0%
Sema.NumFunctionsTypechecked	10,695	10,695	0	0.0%
Sema.NumGenericSignatureBuilders	6,462	6,462	0	0.0%
Sema.NumLazyGenericEnvironments	33,666	33,666	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	4,531	4,531	0	0.0%
Sema.NumLazyIterableDeclContexts	21,372	21,372	0	0.0%
Sema.NumTypesDeserialized	278,753	278,753	0	0.0%
Sema.NumTypesValidated	34,802	34,802	0	0.0%

---

[jrose-apple]

Changing a field is dangerous because if someone passes a lambda expression it won't work. I know some of the SourceKit folks have hit this in the past and have decided that it's better to just use the std::function so that it won't regress in the future. (It should be safe for all the regular function arguments, though.)

[huonw]

This particular field is only used inside another function that passes a function_ref it receives as a parameter, so I think it's okay, but I can revert this if you think it's not worth the risk of being misused in future (although this code hasn't been touched for 3 years).

[jrose-apple]

Yeah, I assumed that was the case. I'm on the fence about it, but I know elsewhere in the codebase we use std::function even though we don't really need to in the same way, just because it's safer. I feel like until we have a good way to catch the use of lambda expressions, we should stick with the safe idiom unless it's a noticeable performance hit.

[huonw]

@swift-ci please smoke test

[huonw]

(For future reference, the ASAN test and full test above both passed, and the only change from then was deleting the modification to WalkToVarDecls Jordan commented on above.)