[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.
This commit is contained in:
Doug Gregor
2020-10-22 13:01:23 -07:00
parent 8521453af3
commit 16104d8045
8 changed files with 86 additions and 38 deletions

View File

@@ -442,7 +442,8 @@ public:
///
/// \param allPropertyNames The set of property names in the enclosing context.
///
/// \param isAsync Whether this is a function imported as 'async'.
/// \param completionHandlerIndex For an 'async' function, the index of the
/// completion handler in argNames.
///
/// \param scratch Scratch space that will be used for modifications beyond
/// just chopping names.
@@ -457,9 +458,13 @@ bool omitNeedlessWords(StringRef &baseName,
bool returnsSelf,
bool isProperty,
const InheritedNameSet *allPropertyNames,
bool isAsync,
Optional<unsigned> completionHandlerIndex,
Optional<StringRef> completionHandlerName,
StringScratchSpace &scratch);
/// If the name has a completion-handler suffix, strip off that suffix.
Optional<StringRef> stripWithCompletionHandlerSuffix(StringRef name);
} // end namespace swift
#endif // SWIFT_BASIC_STRINGEXTRAS_H

View File

@@ -1220,7 +1220,8 @@ bool swift::omitNeedlessWords(StringRef &baseName,
bool returnsSelf,
bool isProperty,
const InheritedNameSet *allPropertyNames,
bool isAsync,
Optional<unsigned> completionHandlerIndex,
Optional<StringRef> completionHandlerName,
StringScratchSpace &scratch) {
bool anyChanges = false;
OmissionTypeName resultType = returnsSelf ? contextType : givenResultType;
@@ -1290,6 +1291,7 @@ bool swift::omitNeedlessWords(StringRef &baseName,
// If the base name of a method imported as "async" starts with the word
// "get", drop the "get".
bool isAsync = completionHandlerIndex.hasValue();
if (isAsync && camel_case::getFirstWord(baseName) == "get" &&
baseName.size() > 3) {
baseName = baseName.substr(3);
@@ -1301,6 +1303,15 @@ bool swift::omitNeedlessWords(StringRef &baseName,
splitBaseName(baseName, argNames[0], paramTypes[0], firstParamName))
anyChanges = true;
// If this is an asynchronous function where the completion handler is
// the first parameter, strip off WithCompletion(Handler) from the base name.
if (isAsync && *completionHandlerIndex == 0) {
if (auto newBaseName = stripWithCompletionHandlerSuffix(baseName)) {
baseName = *newBaseName;
anyChanges = true;
}
}
// For a method imported as "async", drop the "Asynchronously" suffix from
// the base name. It is redundant with 'async'.
const StringRef asynchronously = "Asynchronously";
@@ -1310,6 +1321,21 @@ bool swift::omitNeedlessWords(StringRef &baseName,
anyChanges = true;
}
// If this is an asynchronous function where the completion handler is
// the second parameter, and the corresponding name has some additional
// information prior to WithCompletion(Handler), append that
// additional text to the base name.
if (isAsync && *completionHandlerIndex == 1 && completionHandlerName) {
if (auto extraParamText = stripWithCompletionHandlerSuffix(
*completionHandlerName)) {
SmallString<32> newBaseName;
newBaseName += baseName;
appendSentenceCase(newBaseName, *extraParamText);
baseName = scratch.copyString(newBaseName);
anyChanges = true;
}
}
// Omit needless words based on parameter types.
for (unsigned i = 0, n = argNames.size(); i != n; ++i) {
// If there is no corresponding parameter, there is nothing to
@@ -1329,6 +1355,20 @@ bool swift::omitNeedlessWords(StringRef &baseName,
name, paramTypes[i], role,
role == NameRole::BaseName ? allPropertyNames : nullptr);
// If this is an asynchronous function where the completion handler is
// past the second parameter and has additional information in the name,
// add that information to the prior argument name.
if (isAsync && completionHandlerName && *completionHandlerIndex > 1 &&
*completionHandlerIndex == i + 1) {
if (auto extraParamText = stripWithCompletionHandlerSuffix(
*completionHandlerName)) {
SmallString<32> extendedName;
extendedName += newName;
appendSentenceCase(extendedName, *extraParamText);
newName = scratch.copyString(extendedName);
}
}
if (name == newName) continue;
// Record this change.
@@ -1342,3 +1382,15 @@ bool swift::omitNeedlessWords(StringRef &baseName,
return lowercaseAcronymsForReturn();
}
Optional<StringRef> swift::stripWithCompletionHandlerSuffix(StringRef name) {
if (name.endswith("WithCompletionHandler")) {
return name.drop_back(strlen("WithCompletionHandler"));
}
if (name.endswith("WithCompletion")) {
return name.drop_back(strlen("WithCompletion"));
}
return None;
}

View File

@@ -448,7 +448,7 @@ private:
baseName = humbleBaseName.userFacingName();
bool didOmit =
omitNeedlessWords(baseName, argStrs, "", "", "", paramTypeNames, false,
false, nullptr, false, scratch);
false, nullptr, None, None, scratch);
SmallVector<Identifier, 8> argLabels;
for (auto str : argStrs)
argLabels.push_back(getIdentifier(str));

View File

@@ -804,7 +804,8 @@ static bool omitNeedlessWordsInFunctionName(
ArrayRef<const clang::ParmVarDecl *> params, clang::QualType resultType,
const clang::DeclContext *dc, const SmallBitVector &nonNullArgs,
Optional<unsigned> errorParamIndex, bool returnsSelf, bool isInstanceMethod,
Optional<unsigned> completionHandlerIndex, NameImporter &nameImporter) {
Optional<unsigned> completionHandlerIndex,
Optional<StringRef> completionHandlerName, NameImporter &nameImporter) {
clang::ASTContext &clangCtx = nameImporter.getClangContext();
// Collect the parameter type names.
@@ -854,8 +855,8 @@ static bool omitNeedlessWordsInFunctionName(
getClangTypeNameForOmission(clangCtx, resultType),
getClangTypeNameForOmission(clangCtx, contextType),
paramTypes, returnsSelf, /*isProperty=*/false,
allPropertyNames, completionHandlerIndex.hasValue(),
nameImporter.getScratch());
allPropertyNames, completionHandlerIndex,
completionHandlerName, nameImporter.getScratch());
}
/// Prepare global name for importing onto a swift_newtype.
@@ -1135,26 +1136,9 @@ Optional<ForeignErrorConvention::Info> NameImporter::considerErrorImport(
/// Whether the given parameter name identifies a completion handler.
static bool isCompletionHandlerParamName(StringRef paramName) {
return paramName == "completionHandler" || paramName == "completion" ||
paramName == "withCompletionHandler";
paramName == "withCompletionHandler" || paramName == "withCompletion";
}
/// Whether the give base name implies that the first parameter is a completion
/// handler.
///
/// \returns a trimmed base name when it does, \c None others
static Optional<StringRef> isCompletionHandlerInBaseName(StringRef basename) {
if (basename.endswith("WithCompletionHandler")) {
return basename.drop_back(strlen("WithCompletionHandler"));
}
if (basename.endswith("WithCompletion")) {
return basename.drop_back(strlen("WithCompletion"));
}
return None;
}
// Determine whether the given type is a nullable NSError type.
static bool isNullableNSErrorType(
clang::ASTContext &clangCtx, clang::QualType type) {
@@ -1185,7 +1169,7 @@ static bool isNullableNSErrorType(
Optional<ForeignAsyncConvention::Info>
NameImporter::considerAsyncImport(
const clang::ObjCMethodDecl *clangDecl,
StringRef &baseName,
StringRef baseName,
SmallVectorImpl<StringRef> &paramNames,
ArrayRef<const clang::ParmVarDecl *> params,
bool isInitializer, bool hasCustomName,
@@ -1207,12 +1191,14 @@ NameImporter::considerAsyncImport(
// Determine whether the naming indicates that this is a completion
// handler.
Optional<StringRef> newBaseName;
if (isCompletionHandlerParamName(
paramNames[completionHandlerParamNameIndex])) {
paramNames[completionHandlerParamNameIndex]) ||
(completionHandlerParamNameIndex > 0 &&
stripWithCompletionHandlerSuffix(
paramNames[completionHandlerParamNameIndex]))) {
// The argument label itself has an appropriate name.
} else if (!hasCustomName && completionHandlerParamIndex == 0 &&
(newBaseName = isCompletionHandlerInBaseName(baseName))) {
stripWithCompletionHandlerSuffix(baseName)) {
// The base name implies that the first parameter is a completion handler.
} else if (isCompletionHandlerParamName(
params[completionHandlerParamIndex]->getName())) {
@@ -1301,10 +1287,6 @@ NameImporter::considerAsyncImport(
// Drop the completion handler parameter name.
paramNames.erase(paramNames.begin() + completionHandlerParamNameIndex);
// Update the base name, if needed.
if (newBaseName && !hasCustomName)
baseName = *newBaseName;
return ForeignAsyncConvention::Info(
completionHandlerParamIndex, completionHandlerErrorParamIndex);
}
@@ -1954,7 +1936,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
(void)omitNeedlessWords(baseName, {}, "", propertyTypeName,
contextTypeName, {}, /*returnsSelf=*/false,
/*isProperty=*/true, allPropertyNames,
/*isAsync=*/false, scratch);
None, None, scratch);
}
}
@@ -1971,6 +1953,11 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
[](const ForeignAsyncConvention::Info &info) {
return info.CompletionHandlerParamIndex;
}),
result.getAsyncInfo().map(
[&](const ForeignAsyncConvention::Info &info) {
return method->getDeclName().getObjCSelector().getNameForSlot(
info.CompletionHandlerParamIndex);
}),
*this);
}

View File

@@ -455,7 +455,7 @@ private:
Optional<ForeignAsyncConvention::Info>
considerAsyncImport(const clang::ObjCMethodDecl *clangDecl,
StringRef &baseName,
StringRef baseName,
SmallVectorImpl<StringRef> &paramNames,
ArrayRef<const clang::ParmVarDecl *> params,
bool isInitializer, bool hasCustomName,

View File

@@ -4868,7 +4868,7 @@ Optional<DeclName> TypeChecker::omitNeedlessWords(AbstractFunctionDecl *afd) {
getTypeNameForOmission(contextType),
paramTypes, returnsSelf, false,
/*allPropertyNames=*/nullptr,
afd->hasAsync(), scratch))
None, None, scratch))
return None;
/// Retrieve a replacement identifier.
@@ -4923,8 +4923,7 @@ Optional<Identifier> TypeChecker::omitNeedlessWords(VarDecl *var) {
OmissionTypeName contextTypeName = getTypeNameForOmission(contextType);
if (::omitNeedlessWords(name, { }, "", typeName, contextTypeName, { },
/*returnsSelf=*/false, true,
/*allPropertyNames=*/nullptr, /*isAsync=*/false,
scratch)) {
/*allPropertyNames=*/nullptr, None, None, scratch)) {
return Context.getIdentifier(name);
}

View File

@@ -21,6 +21,9 @@ func testSlowServer(slowServer: SlowServer) async throws {
let _: String? = await try slowServer.fortune()
let _: Int = await try slowServer.magicNumber(withSeed: 42)
await slowServer.serverRestart("localhost")
await slowServer.server("localhost", atPriorityRestart: 0.8)
}
func testSlowServerSynchronous(slowServer: SlowServer) {

View File

@@ -16,6 +16,8 @@
-(void)doSomethingConflicted:(NSString *)operation completionHandler:(void (^)(NSInteger))handler;
-(NSInteger)doSomethingConflicted:(NSString *)operation;
-(void)server:(NSString *)name restartWithCompletionHandler:(void (^)(void))block;
-(void)server:(NSString *)name atPriority:(double)priority restartWithCompletionHandler:(void (^)(void))block;
@end
@protocol RefrigeratorDelegate<NSObject>