Skip to content

Commit b094758

Browse files
DougGregorDougGregor
authored
Merge pull request #34394 from DougGregor/concurrency-objc-name-adjustments
[Concurrency] Don't lose name information from completion-handler arguments
2 parents 33771cf + 16104d8 commit b094758

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
}
@@ -1968,7 +1950,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
19681950
(void)omitNeedlessWords(baseName, {}, "", propertyTypeName,
19691951
contextTypeName, {}, /*returnsSelf=*/false,
19701952
/*isProperty=*/true, allPropertyNames,
1971-
/*isAsync=*/false, scratch);
1953+
None, None, scratch);
19721954
}
19731955
}
19741956

@@ -1985,6 +1967,11 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
19851967
[](const ForeignAsyncConvention::Info &info) {
19861968
return info.CompletionHandlerParamIndex;
19871969
}),
1970+
result.getAsyncInfo().map(
1971+
[&](const ForeignAsyncConvention::Info &info) {
1972+
return method->getDeclName().getObjCSelector().getNameForSlot(
1973+
info.CompletionHandlerParamIndex);
1974+
}),
19881975
*this);
19891976
}
19901977

‎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
@@ -22,6 +22,9 @@ func testSlowServer(slowServer: SlowServer) async throws {
2222

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

2730
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)