Skip to content

Commit 51bace6

Browse files
author
Nathan Hawes
committed
[IDE][SourceKit/DocSupport] Add members of underscored protocol extensions in extensions of conforming types.
We would previously hide the protocol, its extensions and members, but the '_' prefix really just means the protocol itself isn't intended for clients, rather than its members. This also adds support for 'fully_annotated_decl' entries in doc-info for extensions to be consistent with every other decl, and removes the 'fully_annotated_generic_signature' entry we supplied as a fallback. Also fixes several bugs with the synthesized extensions mechanism: - The type sustitutions applied to the extension's requirements were computed using the extension itself as the decl context rather than the extension's nominal. The meant the extension's requirements themselves were assumed to hold when determining the substitutions, so equality constraints were always met. Because of this extension members were incorrectly merged with the base nominal or its extensions despite having additional constraints. - Types within the requirements weren't being transformed when printed (e.g. 'Self.Element' was printed rather than 'T') both in the interface output and in the requirements list. We were also incorrectly printing requirements that were already satisfied once the base type was subsituted in. - If both the protocol extension and 'enabling' extension of the base nominal that added the protocol conformance had conditional requirements, we were only printing the protocol extension's requirements in the synthesized extension. - The USR and annotated decl output embedded in the 'key.doc.full_as_xml' string for synthesized members were printed to match their original context, rather than the synthesized one. Resolves rdar://problem/57121937
1 parent 67d8be7 commit 51bace6

15 files changed

+3106
-1044
lines changed

include/swift/IDE/CommentConversion.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#define SWIFT_IDE_COMMENT_CONVERSION_H
1515

1616
#include "swift/Basic/LLVM.h"
17+
#include "swift/AST/TypeOrExtensionDecl.h"
1718
#include <memory>
1819
#include <string>
1920

@@ -27,7 +28,9 @@ namespace ide {
2728
/// in Clang-like XML format.
2829
///
2930
/// \returns true if the declaration has a documentation comment.
30-
bool getDocumentationCommentAsXML(const Decl *D, raw_ostream &OS);
31+
bool getDocumentationCommentAsXML(
32+
const Decl *D, raw_ostream &OS,
33+
TypeOrExtensionDecl SynthesizedTarget = TypeOrExtensionDecl());
3134

3235
/// If the declaration has a documentation comment and a localization key,
3336
/// print it into the given output stream and return true. Else, return false.

lib/AST/ASTPrinter.cpp

Lines changed: 84 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,10 @@ class PrintAST : public ASTVisitor<PrintAST> {
813813
TypeArrayView<GenericTypeParamType> genericParams,
814814
ArrayRef<Requirement> requirements, unsigned flags,
815815
llvm::function_ref<bool(const Requirement &)> filter);
816+
void printSingleDepthOfGenericSignature(
817+
TypeArrayView<GenericTypeParamType> genericParams,
818+
ArrayRef<Requirement> requirements, bool &isFirstReq, unsigned flags,
819+
llvm::function_ref<bool(const Requirement &)> filter);
816820
void printRequirement(const Requirement &req);
817821

818822
private:
@@ -872,7 +876,12 @@ class PrintAST : public ASTVisitor<PrintAST> {
872876
return false; // not needed for the parser library.
873877
#endif
874878

875-
if (!shouldPrint(D, true))
879+
bool Synthesize =
880+
Options.TransformContext &&
881+
Options.TransformContext->isPrintingSynthesizedExtension() &&
882+
isa<ExtensionDecl>(D);
883+
884+
if (!shouldPrint(D, true) && !Synthesize)
876885
return false;
877886

878887
Decl *Old = Current;
@@ -890,10 +899,6 @@ class PrintAST : public ASTVisitor<PrintAST> {
890899

891900
SWIFT_DEFER { CurrentType = OldType; };
892901

893-
bool Synthesize =
894-
Options.TransformContext &&
895-
Options.TransformContext->isPrintingSynthesizedExtension() &&
896-
isa<ExtensionDecl>(D);
897902
if (Synthesize) {
898903
Printer.setSynthesizedTarget(Options.TransformContext->getDecl());
899904
}
@@ -1456,6 +1461,15 @@ void PrintAST::printSingleDepthOfGenericSignature(
14561461
TypeArrayView<GenericTypeParamType> genericParams,
14571462
ArrayRef<Requirement> requirements, unsigned flags,
14581463
llvm::function_ref<bool(const Requirement &)> filter) {
1464+
bool isFirstReq = true;
1465+
printSingleDepthOfGenericSignature(genericParams, requirements, isFirstReq,
1466+
flags, filter);
1467+
}
1468+
1469+
void PrintAST::printSingleDepthOfGenericSignature(
1470+
TypeArrayView<GenericTypeParamType> genericParams,
1471+
ArrayRef<Requirement> requirements, bool &isFirstReq, unsigned flags,
1472+
llvm::function_ref<bool(const Requirement &)> filter) {
14591473
bool printParams = (flags & PrintParams);
14601474
bool printRequirements = (flags & PrintRequirements);
14611475
printRequirements &= Options.PrintGenericRequirements;
@@ -1502,7 +1516,6 @@ void PrintAST::printSingleDepthOfGenericSignature(
15021516
}
15031517

15041518
if (printRequirements || printInherited) {
1505-
bool isFirstReq = true;
15061519
for (const auto &req : requirements) {
15071520
if (!filter(req))
15081521
continue;
@@ -1564,9 +1577,6 @@ void PrintAST::printSingleDepthOfGenericSignature(
15641577
}
15651578
} else {
15661579
Printer.callPrintStructurePre(PrintStructureKind::GenericRequirement);
1567-
1568-
// We don't substitute type for the printed requirement so that the
1569-
// printed requirement agrees with separately reported generic parameters.
15701580
printRequirement(req);
15711581
Printer.printStructurePost(PrintStructureKind::GenericRequirement);
15721582
}
@@ -1578,7 +1588,7 @@ void PrintAST::printSingleDepthOfGenericSignature(
15781588
}
15791589

15801590
void PrintAST::printRequirement(const Requirement &req) {
1581-
printType(req.getFirstType());
1591+
printTransformedType(req.getFirstType());
15821592
switch (req.getKind()) {
15831593
case RequirementKind::Layout:
15841594
Printer << " : ";
@@ -1592,7 +1602,7 @@ void PrintAST::printRequirement(const Requirement &req) {
15921602
Printer << " == ";
15931603
break;
15941604
}
1595-
printType(req.getSecondType());
1605+
printTransformedType(req.getSecondType());
15961606
}
15971607

15981608
bool PrintAST::shouldPrintPattern(const Pattern *P) {
@@ -2180,16 +2190,78 @@ static void printExtendedTypeName(Type ExtendedType, ASTPrinter &Printer,
21802190
Ty->print(Printer, Options);
21812191
}
21822192

2193+
21832194
void PrintAST::printSynthesizedExtension(Type ExtendedType,
21842195
ExtensionDecl *ExtDecl) {
2196+
2197+
auto printRequirementsFrom = [&](ExtensionDecl *ED, bool &IsFirst) {
2198+
auto Sig = ED->getGenericSignature();
2199+
printSingleDepthOfGenericSignature(Sig->getGenericParams(),
2200+
Sig->getRequirements(),
2201+
IsFirst, PrintRequirements,
2202+
[](const Requirement &Req){
2203+
return true;
2204+
});
2205+
};
2206+
2207+
auto printCombinedRequirementsIfNeeded = [&]() -> bool {
2208+
if (!Options.TransformContext ||
2209+
!Options.TransformContext->isPrintingSynthesizedExtension())
2210+
return false;
2211+
2212+
// Combined requirements only needed if the transform context is an enabling
2213+
// extension of the protocol rather than a nominal (which can't have
2214+
// constraints of its own).
2215+
ExtensionDecl *Target = dyn_cast<ExtensionDecl>(
2216+
Options.TransformContext->getDecl().getAsDecl());
2217+
if (!Target || Target == ExtDecl)
2218+
return false;
2219+
2220+
bool IsFirst = true;
2221+
if (ExtDecl->isConstrainedExtension()) {
2222+
printRequirementsFrom(ExtDecl, IsFirst);
2223+
}
2224+
if (Target->isConstrainedExtension()) {
2225+
if (auto *NTD = Target->getExtendedNominal()) {
2226+
// Update the current decl and type transform for Target rather than
2227+
// ExtDecl.
2228+
PrintOptions Adjusted = Options;
2229+
Adjusted.initForSynthesizedExtension(NTD);
2230+
llvm::SaveAndRestore<Decl*> TempCurrent(Current, NTD);
2231+
llvm::SaveAndRestore<PrintOptions> TempOptions(Options, Adjusted);
2232+
printRequirementsFrom(Target, IsFirst);
2233+
}
2234+
}
2235+
return true;
2236+
};
2237+
2238+
21852239
if (Options.BracketOptions.shouldOpenExtension(ExtDecl)) {
21862240
printDocumentationComment(ExtDecl);
21872241
printAttributes(ExtDecl);
21882242
Printer << tok::kw_extension << " ";
21892243

21902244
printExtendedTypeName(ExtendedType, Printer, Options);
21912245
printInherited(ExtDecl);
2192-
printDeclGenericRequirements(ExtDecl);
2246+
2247+
// We may need to combine requirements from ExtDecl (which has the members
2248+
// to print) and the TransformContexts' decl if it is an enabling extension
2249+
// of the base NominalDecl (which can have its own requirements) rather than
2250+
// base NominalDecl itself (which can't). E.g:
2251+
//
2252+
// protocol Foo {}
2253+
// extension Foo where <requirments from ExtDecl> { ... }
2254+
// struct Bar {}
2255+
// extension Bar: Foo where <requirments from TransformContext> { ... }
2256+
//
2257+
// should produce a synthesized extension of Bar with both sets of
2258+
// requirments:
2259+
//
2260+
// extension Bar where <requirments from ExtDecl+TransformContext { ... }
2261+
//
2262+
if (!printCombinedRequirementsIfNeeded())
2263+
printDeclGenericRequirements(ExtDecl);
2264+
21932265
}
21942266
if (Options.TypeDefinitions) {
21952267
printMembersOfDecl(ExtDecl, false,

lib/IDE/CommentConversion.cpp

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ struct CommentToXMLConverter {
258258
OS << "</Tags>";
259259
}
260260

261-
void visitDocComment(const DocComment *DC);
261+
void visitDocComment(const DocComment *DC, TypeOrExtensionDecl SynthesizedTarget);
262262
void visitCommentParts(const swift::markup::CommentParts &Parts);
263263
};
264264
} // unnamed namespace
@@ -297,7 +297,8 @@ void CommentToXMLConverter::visitCommentParts(const swift::markup::CommentParts
297297
}
298298
}
299299

300-
void CommentToXMLConverter::visitDocComment(const DocComment *DC) {
300+
void CommentToXMLConverter::
301+
visitDocComment(const DocComment *DC, TypeOrExtensionDecl SynthesizedTarget) {
301302
const Decl *D = DC->getDecl();
302303

303304
StringRef RootEndTag;
@@ -347,6 +348,10 @@ void CommentToXMLConverter::visitDocComment(const DocComment *DC) {
347348
{
348349
llvm::raw_svector_ostream OS(SS);
349350
Failed = ide::printValueDeclUSR(VD, OS);
351+
if (!Failed && SynthesizedTarget) {
352+
OS << "::SYNTHESIZED::";
353+
Failed = ide::printValueDeclUSR(SynthesizedTarget.getBaseNominal(), OS);
354+
}
350355
}
351356
if (!Failed && !SS.empty()) {
352357
OS << "<USR>" << SS << "</USR>";
@@ -362,6 +367,9 @@ void CommentToXMLConverter::visitDocComment(const DocComment *DC) {
362367
PO.VarInitializers = false;
363368
PO.ShouldQualifyNestedDeclarations =
364369
PrintOptions::QualifyNestedDeclarations::TypesOnly;
370+
PO.SkipUnderscoredStdlibProtocols = false;
371+
if (SynthesizedTarget)
372+
PO.initForSynthesizedExtension(SynthesizedTarget);
365373

366374
OS << "<Declaration>";
367375
llvm::SmallString<32> DeclSS;
@@ -398,12 +406,15 @@ static bool getClangDocumentationCommentAsXML(const clang::Decl *D,
398406
return true;
399407
}
400408

401-
static void replaceObjcDeclarationsWithSwiftOnes(const Decl *D,
402-
StringRef Doc,
403-
raw_ostream &OS) {
409+
static void
410+
replaceObjcDeclarationsWithSwiftOnes(const Decl *D, StringRef Doc,
411+
raw_ostream &OS,
412+
TypeOrExtensionDecl SynthesizedTarget) {
404413
StringRef Open = "<Declaration>";
405414
StringRef Close = "</Declaration>";
406415
PrintOptions Options = PrintOptions::printQuickHelpDeclaration();
416+
if (SynthesizedTarget)
417+
Options.initForSynthesizedExtension(SynthesizedTarget);
407418
std::string S;
408419
llvm::raw_string_ostream SS(S);
409420
D->print(SS, Options);
@@ -444,14 +455,16 @@ std::string ide::extractPlainTextFromComment(const StringRef Text) {
444455
return getLineListFromComment(SourceMgr, MC, Text).str();
445456
}
446457

447-
bool ide::getDocumentationCommentAsXML(const Decl *D, raw_ostream &OS) {
458+
bool ide::getDocumentationCommentAsXML(const Decl *D, raw_ostream &OS,
459+
TypeOrExtensionDecl SynthesizedTarget) {
448460
auto MaybeClangNode = D->getClangNode();
449461
if (MaybeClangNode) {
450462
if (auto *CD = MaybeClangNode.getAsDecl()) {
451463
std::string S;
452464
llvm::raw_string_ostream SS(S);
453465
if (getClangDocumentationCommentAsXML(CD, SS)) {
454-
replaceObjcDeclarationsWithSwiftOnes(D, SS.str(), OS);
466+
replaceObjcDeclarationsWithSwiftOnes(D, SS.str(), OS,
467+
SynthesizedTarget);
455468
return true;
456469
}
457470
}
@@ -464,7 +477,7 @@ bool ide::getDocumentationCommentAsXML(const Decl *D, raw_ostream &OS) {
464477
return false;
465478

466479
CommentToXMLConverter Converter(OS);
467-
Converter.visitDocComment(DC);
480+
Converter.visitDocComment(DC, SynthesizedTarget);
468481

469482
OS.flush();
470483
return true;

lib/IDE/IDETypeChecking.cpp

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -282,16 +282,27 @@ struct SynthesizedExtensionAnalyzer::Implementation {
282282

283283
auto handleRequirements = [&](SubstitutionMap subMap,
284284
GenericSignature GenericSig,
285+
ExtensionDecl *OwningExt,
285286
ArrayRef<Requirement> Reqs) {
287+
ProtocolDecl *BaseProto = OwningExt->getInnermostDeclContext()
288+
->getSelfProtocolDecl();
286289
for (auto Req : Reqs) {
287290
auto Kind = Req.getKind();
288291

289292
// FIXME: Could do something here
290293
if (Kind == RequirementKind::Layout)
291294
continue;
292295

293-
auto First = Req.getFirstType();
294-
auto Second = Req.getSecondType();
296+
Type First = Req.getFirstType();
297+
Type Second = Req.getSecondType();
298+
299+
// Skip protocol's Self : <Protocol> requirement.
300+
if (BaseProto &&
301+
Req.getKind() == RequirementKind::Conformance &&
302+
First->isEqual(BaseProto->getSelfInterfaceType()) &&
303+
Second->getAnyNominal() == BaseProto)
304+
continue;
305+
295306
if (!BaseType->isExistentialType()) {
296307
First = First.subst(subMap);
297308
Second = Second.subst(subMap);
@@ -346,19 +357,29 @@ struct SynthesizedExtensionAnalyzer::Implementation {
346357
// the extension to the interface types of the base type's
347358
// declaration.
348359
SubstitutionMap subMap;
349-
if (!BaseType->isExistentialType())
350-
subMap = BaseType->getContextSubstitutionMap(M, Ext);
360+
if (!BaseType->isExistentialType()) {
361+
if (auto *NTD = Ext->getExtendedNominal())
362+
subMap = BaseType->getContextSubstitutionMap(M, NTD);
363+
}
351364

352365
assert(Ext->getGenericSignature() && "No generic signature.");
353366
auto GenericSig = Ext->getGenericSignature();
354-
if (handleRequirements(subMap, GenericSig, GenericSig->getRequirements()))
367+
if (handleRequirements(subMap, GenericSig, Ext, GenericSig->getRequirements()))
355368
return {Result, MergeInfo};
356369
}
357370

358-
if (Conf && handleRequirements(Conf->getSubstitutions(M),
359-
Conf->getGenericSignature(),
360-
Conf->getConditionalRequirements()))
361-
return {Result, MergeInfo};
371+
if (Conf) {
372+
SubstitutionMap subMap;
373+
if (!BaseType->isExistentialType()) {
374+
if (auto *NTD = EnablingExt->getExtendedNominal())
375+
subMap = BaseType->getContextSubstitutionMap(M, NTD);
376+
}
377+
if (handleRequirements(subMap,
378+
Conf->getGenericSignature(),
379+
EnablingExt,
380+
Conf->getConditionalRequirements()))
381+
return {Result, MergeInfo};
382+
}
362383

363384
Result.Ext = Ext;
364385
return {Result, MergeInfo};
@@ -431,7 +452,14 @@ struct SynthesizedExtensionAnalyzer::Implementation {
431452
auto handleExtension = [&](ExtensionDecl *E, bool Synthesized,
432453
ExtensionDecl *EnablingE,
433454
NormalProtocolConformance *Conf) {
434-
if (Options.shouldPrint(E)) {
455+
PrintOptions AdjustedOpts = Options;
456+
if (Synthesized) {
457+
// Members from underscored system protocols should still appear as
458+
// members of the target type, even if the protocols themselves are not
459+
// printed.
460+
AdjustedOpts.SkipUnderscoredStdlibProtocols = false;
461+
}
462+
if (AdjustedOpts.shouldPrint(E)) {
435463
auto Pair = isApplicable(E, Synthesized, EnablingE, Conf);
436464
if (Pair.first) {
437465
InfoMap->insert({E, Pair.first});

test/IDE/print_synthesized_extensions.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,21 +237,21 @@ extension S13 : P5 {
237237
public func foo1() {}
238238
}
239239

240-
// CHECK1: <synthesized>extension <ref:Struct>S1</ref> where <ref:GenericTypeParam>Self</ref>.P2T1 : <ref:Protocol>P2</ref> {
240+
// CHECK1: <synthesized>extension <ref:Struct>S1</ref> where T : <ref:Protocol>P2</ref> {
241241
// CHECK1-NEXT: <decl:Func>public func <loc>p2member()</loc></decl>
242242
// CHECK1-NEXT: <decl:Func>public func <loc>ef1(<decl:Param>t: T</decl>)</loc></decl>
243243
// CHECK1-NEXT: <decl:Func>public func <loc>ef2(<decl:Param>t: <ref:Struct>S2</ref></decl>)</loc></decl>
244244
// CHECK1-NEXT: }</synthesized>
245245

246-
// CHECK2: <synthesized>extension <ref:Struct>S1</ref> where <ref:GenericTypeParam>Self</ref>.T1 : <ref:Protocol>P3</ref> {
246+
// CHECK2: <synthesized>extension <ref:Struct>S1</ref> where T : <ref:Protocol>P3</ref> {
247247
// CHECK2-NEXT: <decl:Func>public func <loc>p3Func(<decl:Param>i: <ref:Struct>Int</ref></decl>)</loc> -> <ref:Struct>Int</ref></decl>
248248
// CHECK2-NEXT: }</synthesized>
249249

250-
// CHECK3: <synthesized>extension <ref:Struct>S1</ref> where <ref:GenericTypeParam>Self</ref>.T1 == <ref:Struct>Int</ref> {
250+
// CHECK3: <synthesized>extension <ref:Struct>S1</ref> where T == <ref:Struct>Int</ref> {
251251
// CHECK3-NEXT: <decl:Func>public func <loc>p1IntFunc(<decl:Param>i: <ref:Struct>Int</ref></decl>)</loc> -> <ref:Struct>Int</ref></decl>
252252
// CHECK3-NEXT: }</synthesized>
253253

254-
// CHECK4: <synthesized>extension <ref:Struct>S1</ref> where <ref:GenericTypeParam>Self</ref>.T1 == <ref:Struct>S9</ref><<ref:Struct>Int</ref>> {
254+
// CHECK4: <synthesized>extension <ref:Struct>S1</ref> where T == <ref:Struct>S9</ref><<ref:Struct>Int</ref>> {
255255
// CHECK4-NEXT: <decl:Func>public func <loc>S9IntFunc()</loc></decl>
256256
// CHECK4-NEXT: }</synthesized>
257257

0 commit comments

Comments
 (0)