Skip to content

Commit 16104d8

Browse files
DougGregorDougGregor
committed
[Concurrency] Don't lose name information from completion-handler arguments.
When a completion handler parameter has a selector piece that ends with "WithCompletion(Handler)", prepend the text before that suffix to the base name or previous argument label, as appropriate. This ensures that we don't lose information from the name, particularly with delegate names.
1 parent 8521453 commit 16104d8

File tree

8 files changed

+86
-38
lines changed

8 files changed

+86
-38
lines changed

‎include/swift/Basic/StringExtras.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,8 @@ class InheritedNameSet {
442442
///
443443
/// \param allPropertyNames The set of property names in the enclosing context.
444444
///
445-
/// \param isAsync Whether this is a function imported as 'async'.
445+
/// \param completionHandlerIndex For an 'async' function, the index of the
446+
/// completion handler in argNames.
446447
///
447448
/// \param scratch Scratch space that will be used for modifications beyond
448449
/// just chopping names.
@@ -457,9 +458,13 @@ bool omitNeedlessWords(StringRef &baseName,
457458
bool returnsSelf,
458459
bool isProperty,
459460
const InheritedNameSet *allPropertyNames,
460-
bool isAsync,
461+
Optional<unsigned> completionHandlerIndex,
462+
Optional<StringRef> completionHandlerName,
461463
StringScratchSpace &scratch);
462464

465+
/// If the name has a completion-handler suffix, strip off that suffix.
466+
Optional<StringRef> stripWithCompletionHandlerSuffix(StringRef name);
467+
463468
} // end namespace swift
464469

465470
#endif // SWIFT_BASIC_STRINGEXTRAS_H

‎lib/Basic/StringExtras.cpp

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1220,7 +1220,8 @@ bool swift::omitNeedlessWords(StringRef &baseName,
12201220
bool returnsSelf,
12211221
bool isProperty,
12221222
const InheritedNameSet *allPropertyNames,
1223-
bool isAsync,
1223+
Optional<unsigned> completionHandlerIndex,
1224+
Optional<StringRef> completionHandlerName,
12241225
StringScratchSpace &scratch) {
12251226
bool anyChanges = false;
12261227
OmissionTypeName resultType = returnsSelf ? contextType : givenResultType;
@@ -1290,6 +1291,7 @@ bool swift::omitNeedlessWords(StringRef &baseName,
12901291

12911292
// If the base name of a method imported as "async" starts with the word
12921293
// "get", drop the "get".
1294+
bool isAsync = completionHandlerIndex.hasValue();
12931295
if (isAsync && camel_case::getFirstWord(baseName) == "get" &&
12941296
baseName.size() > 3) {
12951297
baseName = baseName.substr(3);
@@ -1301,6 +1303,15 @@ bool swift::omitNeedlessWords(StringRef &baseName,
13011303
splitBaseName(baseName, argNames[0], paramTypes[0], firstParamName))
13021304
anyChanges = true;
13031305

1306+
// If this is an asynchronous function where the completion handler is
1307+
// the first parameter, strip off WithCompletion(Handler) from the base name.
1308+
if (isAsync && *completionHandlerIndex == 0) {
1309+
if (auto newBaseName = stripWithCompletionHandlerSuffix(baseName)) {
1310+
baseName = *newBaseName;
1311+
anyChanges = true;
1312+
}
1313+
}
1314+
13041315
// For a method imported as "async", drop the "Asynchronously" suffix from
13051316
// the base name. It is redundant with 'async'.
13061317
const StringRef asynchronously = "Asynchronously";
@@ -1310,6 +1321,21 @@ bool swift::omitNeedlessWords(StringRef &baseName,
13101321
anyChanges = true;
13111322
}
13121323

1324+
// If this is an asynchronous function where the completion handler is
1325+
// the second parameter, and the corresponding name has some additional
1326+
// information prior to WithCompletion(Handler), append that
1327+
// additional text to the base name.
1328+
if (isAsync && *completionHandlerIndex == 1 && completionHandlerName) {
1329+
if (auto extraParamText = stripWithCompletionHandlerSuffix(
1330+
*completionHandlerName)) {
1331+
SmallString<32> newBaseName;
1332+
newBaseName += baseName;
1333+
appendSentenceCase(newBaseName, *extraParamText);
1334+
baseName = scratch.copyString(newBaseName);
1335+
anyChanges = true;
1336+
}
1337+
}
1338+
13131339
// Omit needless words based on parameter types.
13141340
for (unsigned i = 0, n = argNames.size(); i != n; ++i) {
13151341
// If there is no corresponding parameter, there is nothing to
@@ -1329,6 +1355,20 @@ bool swift::omitNeedlessWords(StringRef &baseName,
13291355
name, paramTypes[i], role,
13301356
role == NameRole::BaseName ? allPropertyNames : nullptr);
13311357

1358+
// If this is an asynchronous function where the completion handler is
1359+
// past the second parameter and has additional information in the name,
1360+
// add that information to the prior argument name.
1361+
if (isAsync && completionHandlerName && *completionHandlerIndex > 1 &&
1362+
*completionHandlerIndex == i + 1) {
1363+
if (auto extraParamText = stripWithCompletionHandlerSuffix(
1364+
*completionHandlerName)) {
1365+
SmallString<32> extendedName;
1366+
extendedName += newName;
1367+
appendSentenceCase(extendedName, *extraParamText);
1368+
newName = scratch.copyString(extendedName);
1369+
}
1370+
}
1371+
13321372
if (name == newName) continue;
13331373

13341374
// Record this change.
@@ -1342,3 +1382,15 @@ bool swift::omitNeedlessWords(StringRef &baseName,
13421382

13431383
return lowercaseAcronymsForReturn();
13441384
}
1385+
1386+
Optional<StringRef> swift::stripWithCompletionHandlerSuffix(StringRef name) {
1387+
if (name.endswith("WithCompletionHandler")) {
1388+
return name.drop_back(strlen("WithCompletionHandler"));
1389+
}
1390+
1391+
if (name.endswith("WithCompletion")) {
1392+
return name.drop_back(strlen("WithCompletion"));
1393+
}
1394+
1395+
return None;
1396+
}

‎lib/ClangImporter/IAMInference.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ class IAMInference {
448448
baseName = humbleBaseName.userFacingName();
449449
bool didOmit =
450450
omitNeedlessWords(baseName, argStrs, "", "", "", paramTypeNames, false,
451-
false, nullptr, false, scratch);
451+
false, nullptr, None, None, scratch);
452452
SmallVector<Identifier, 8> argLabels;
453453
for (auto str : argStrs)
454454
argLabels.push_back(getIdentifier(str));

‎lib/ClangImporter/ImportName.cpp

Lines changed: 17 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,8 @@ static bool omitNeedlessWordsInFunctionName(
804804
ArrayRef<const clang::ParmVarDecl *> params, clang::QualType resultType,
805805
const clang::DeclContext *dc, const SmallBitVector &nonNullArgs,
806806
Optional<unsigned> errorParamIndex, bool returnsSelf, bool isInstanceMethod,
807-
Optional<unsigned> completionHandlerIndex, NameImporter &nameImporter) {
807+
Optional<unsigned> completionHandlerIndex,
808+
Optional<StringRef> completionHandlerName, NameImporter &nameImporter) {
808809
clang::ASTContext &clangCtx = nameImporter.getClangContext();
809810

810811
// Collect the parameter type names.
@@ -854,8 +855,8 @@ static bool omitNeedlessWordsInFunctionName(
854855
getClangTypeNameForOmission(clangCtx, resultType),
855856
getClangTypeNameForOmission(clangCtx, contextType),
856857
paramTypes, returnsSelf, /*isProperty=*/false,
857-
allPropertyNames, completionHandlerIndex.hasValue(),
858-
nameImporter.getScratch());
858+
allPropertyNames, completionHandlerIndex,
859+
completionHandlerName, nameImporter.getScratch());
859860
}
860861

861862
/// Prepare global name for importing onto a swift_newtype.
@@ -1135,26 +1136,9 @@ Optional<ForeignErrorConvention::Info> NameImporter::considerErrorImport(
11351136
/// Whether the given parameter name identifies a completion handler.
11361137
static bool isCompletionHandlerParamName(StringRef paramName) {
11371138
return paramName == "completionHandler" || paramName == "completion" ||
1138-
paramName == "withCompletionHandler";
1139+
paramName == "withCompletionHandler" || paramName == "withCompletion";
11391140
}
11401141

1141-
/// Whether the give base name implies that the first parameter is a completion
1142-
/// handler.
1143-
///
1144-
/// \returns a trimmed base name when it does, \c None others
1145-
static Optional<StringRef> isCompletionHandlerInBaseName(StringRef basename) {
1146-
if (basename.endswith("WithCompletionHandler")) {
1147-
return basename.drop_back(strlen("WithCompletionHandler"));
1148-
}
1149-
1150-
if (basename.endswith("WithCompletion")) {
1151-
return basename.drop_back(strlen("WithCompletion"));
1152-
}
1153-
1154-
return None;
1155-
}
1156-
1157-
11581142
// Determine whether the given type is a nullable NSError type.
11591143
static bool isNullableNSErrorType(
11601144
clang::ASTContext &clangCtx, clang::QualType type) {
@@ -1185,7 +1169,7 @@ static bool isNullableNSErrorType(
11851169
Optional<ForeignAsyncConvention::Info>
11861170
NameImporter::considerAsyncImport(
11871171
const clang::ObjCMethodDecl *clangDecl,
1188-
StringRef &baseName,
1172+
StringRef baseName,
11891173
SmallVectorImpl<StringRef> &paramNames,
11901174
ArrayRef<const clang::ParmVarDecl *> params,
11911175
bool isInitializer, bool hasCustomName,
@@ -1207,12 +1191,14 @@ NameImporter::considerAsyncImport(
12071191

12081192
// Determine whether the naming indicates that this is a completion
12091193
// handler.
1210-
Optional<StringRef> newBaseName;
12111194
if (isCompletionHandlerParamName(
1212-
paramNames[completionHandlerParamNameIndex])) {
1195+
paramNames[completionHandlerParamNameIndex]) ||
1196+
(completionHandlerParamNameIndex > 0 &&
1197+
stripWithCompletionHandlerSuffix(
1198+
paramNames[completionHandlerParamNameIndex]))) {
12131199
// The argument label itself has an appropriate name.
12141200
} else if (!hasCustomName && completionHandlerParamIndex == 0 &&
1215-
(newBaseName = isCompletionHandlerInBaseName(baseName))) {
1201+
stripWithCompletionHandlerSuffix(baseName)) {
12161202
// The base name implies that the first parameter is a completion handler.
12171203
} else if (isCompletionHandlerParamName(
12181204
params[completionHandlerParamIndex]->getName())) {
@@ -1301,10 +1287,6 @@ NameImporter::considerAsyncImport(
13011287
// Drop the completion handler parameter name.
13021288
paramNames.erase(paramNames.begin() + completionHandlerParamNameIndex);
13031289

1304-
// Update the base name, if needed.
1305-
if (newBaseName && !hasCustomName)
1306-
baseName = *newBaseName;
1307-
13081290
return ForeignAsyncConvention::Info(
13091291
completionHandlerParamIndex, completionHandlerErrorParamIndex);
13101292
}
@@ -1954,7 +1936,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
19541936
(void)omitNeedlessWords(baseName, {}, "", propertyTypeName,
19551937
contextTypeName, {}, /*returnsSelf=*/false,
19561938
/*isProperty=*/true, allPropertyNames,
1957-
/*isAsync=*/false, scratch);
1939+
None, None, scratch);
19581940
}
19591941
}
19601942

@@ -1971,6 +1953,11 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
19711953
[](const ForeignAsyncConvention::Info &info) {
19721954
return info.CompletionHandlerParamIndex;
19731955
}),
1956+
result.getAsyncInfo().map(
1957+
[&](const ForeignAsyncConvention::Info &info) {
1958+
return method->getDeclName().getObjCSelector().getNameForSlot(
1959+
info.CompletionHandlerParamIndex);
1960+
}),
19741961
*this);
19751962
}
19761963

‎lib/ClangImporter/ImportName.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ class NameImporter {
455455

456456
Optional<ForeignAsyncConvention::Info>
457457
considerAsyncImport(const clang::ObjCMethodDecl *clangDecl,
458-
StringRef &baseName,
458+
StringRef baseName,
459459
SmallVectorImpl<StringRef> &paramNames,
460460
ArrayRef<const clang::ParmVarDecl *> params,
461461
bool isInitializer, bool hasCustomName,

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4868,7 +4868,7 @@ Optional<DeclName> TypeChecker::omitNeedlessWords(AbstractFunctionDecl *afd) {
48684868
getTypeNameForOmission(contextType),
48694869
paramTypes, returnsSelf, false,
48704870
/*allPropertyNames=*/nullptr,
4871-
afd->hasAsync(), scratch))
4871+
None, None, scratch))
48724872
return None;
48734873

48744874
/// Retrieve a replacement identifier.
@@ -4923,8 +4923,7 @@ Optional<Identifier> TypeChecker::omitNeedlessWords(VarDecl *var) {
49234923
OmissionTypeName contextTypeName = getTypeNameForOmission(contextType);
49244924
if (::omitNeedlessWords(name, { }, "", typeName, contextTypeName, { },
49254925
/*returnsSelf=*/false, true,
4926-
/*allPropertyNames=*/nullptr, /*isAsync=*/false,
4927-
scratch)) {
4926+
/*allPropertyNames=*/nullptr, None, None, scratch)) {
49284927
return Context.getIdentifier(name);
49294928
}
49304929

‎test/ClangImporter/objc_async.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ func testSlowServer(slowServer: SlowServer) async throws {
2121

2222
let _: String? = await try slowServer.fortune()
2323
let _: Int = await try slowServer.magicNumber(withSeed: 42)
24+
25+
await slowServer.serverRestart("localhost")
26+
await slowServer.server("localhost", atPriorityRestart: 0.8)
2427
}
2528

2629
func testSlowServerSynchronous(slowServer: SlowServer) {

‎test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
-(void)doSomethingConflicted:(NSString *)operation completionHandler:(void (^)(NSInteger))handler;
1818
-(NSInteger)doSomethingConflicted:(NSString *)operation;
19+
-(void)server:(NSString *)name restartWithCompletionHandler:(void (^)(void))block;
20+
-(void)server:(NSString *)name atPriority:(double)priority restartWithCompletionHandler:(void (^)(void))block;
1921
@end
2022

2123
@protocol RefrigeratorDelegate<NSObject>

0 commit comments

Comments
 (0)