Skip to content

Commit 92b36a3

Browse files
hamishknighthamishknight
committed
Drop SerializedOK parameter from getRawComment
It doesn't seem like there's any client that's actually taking advantage of setting it to `false`, and its default value of `false` is more likely than not going to cause clients to accidentally miss comments that they may want. In fact, this was exactly the case for code completion's brief field. Finally, the parameter wasn't even consistently applied, as we would attempt to deserialize swiftdoc comments even if it were `false`.
1 parent 86927ef commit 92b36a3

File tree

11 files changed

+52
-73
lines changed

11 files changed

+52
-73
lines changed

‎include/swift/AST/Comment.h

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -93,27 +93,16 @@ class DocComment {
9393
};
9494

9595
/// Get a parsed documentation comment for the declaration, if there is one.
96-
///
97-
/// \param AllowSerialized Allow loading serialized doc comment data, including
98-
/// comment ranges.
9996
DocComment *getSingleDocComment(swift::markup::MarkupContext &Context,
100-
const Decl *D, bool AllowSerialized = false);
97+
const Decl *D);
10198

10299
/// Get the declaration that actually provides a doc comment for another.
103-
///
104-
/// \param AllowSerialized Allow loading serialized doc comment data, including
105-
/// comment ranges.
106-
const Decl *getDocCommentProvidingDecl(const Decl *D,
107-
bool AllowSerialized = false);
100+
const Decl *getDocCommentProvidingDecl(const Decl *D);
108101

109102
/// Attempt to get a doc comment from the declaration, or other inherited
110103
/// sources, like from base classes or protocols.
111-
///
112-
/// \param AllowSerialized Allow loading serialized doc comment data, including
113-
/// comment ranges.
114104
DocComment *getCascadingDocComment(swift::markup::MarkupContext &MC,
115-
const Decl *D,
116-
bool AllowSerialized = false);
105+
const Decl *D);
117106

118107
/// Extract comments parts from the given Markup node.
119108
swift::markup::CommentParts

‎include/swift/AST/Decl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
10151015
}
10161016

10171017
/// \returns the unparsed comment attached to this declaration.
1018-
RawComment getRawComment(bool SerializedOK = false) const;
1018+
RawComment getRawComment() const;
10191019

10201020
Optional<StringRef> getGroupName() const;
10211021

‎lib/AST/DocComment.cpp

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -381,33 +381,31 @@ void DocComment::addInheritanceNote(swift::markup::MarkupContext &MC,
381381
}
382382

383383
DocComment *swift::getSingleDocComment(swift::markup::MarkupContext &MC,
384-
const Decl *D, bool AllowSerialized) {
384+
const Decl *D) {
385385
PrettyStackTraceDecl StackTrace("parsing comment for", D);
386386

387-
auto RC = D->getRawComment(AllowSerialized);
387+
auto RC = D->getRawComment();
388388
if (RC.isEmpty())
389389
return nullptr;
390390
return DocComment::create(D, MC, RC);
391391
}
392392

393393
namespace {
394-
const ValueDecl *findOverriddenDeclWithDocComment(const ValueDecl *VD,
395-
bool AllowSerialized = true) {
394+
const ValueDecl *findOverriddenDeclWithDocComment(const ValueDecl *VD) {
396395
// Only applies to class member.
397396
if (!VD->getDeclContext()->getSelfClassDecl())
398397
return nullptr;
399398

400399
while (auto *baseDecl = VD->getOverriddenDecl()) {
401-
if (!baseDecl->getRawComment(AllowSerialized).isEmpty())
400+
if (!baseDecl->getRawComment().isEmpty())
402401
return baseDecl;
403402
VD = baseDecl;
404403
}
405404

406405
return nullptr;
407406
}
408407

409-
const ValueDecl *findDefaultProvidedDeclWithDocComment(const ValueDecl *VD,
410-
bool AllowSerialized = false) {
408+
const ValueDecl *findDefaultProvidedDeclWithDocComment(const ValueDecl *VD) {
411409
auto protocol = VD->getDeclContext()->getExtendedProtocolDecl();
412410
// Only applies to protocol extension member.
413411
if (!protocol)
@@ -423,8 +421,7 @@ const ValueDecl *findDefaultProvidedDeclWithDocComment(const ValueDecl *VD,
423421

424422
for (auto *member : members) {
425423
if (!isa<ProtocolDecl>(member->getDeclContext()) ||
426-
!member->isProtocolRequirement() ||
427-
member->getRawComment(AllowSerialized).isEmpty())
424+
!member->isProtocolRequirement() || member->getRawComment().isEmpty())
428425
continue;
429426
if (requirement)
430427
// Found two or more decls with doc-comment.
@@ -435,12 +432,11 @@ const ValueDecl *findDefaultProvidedDeclWithDocComment(const ValueDecl *VD,
435432
return requirement;
436433
}
437434

438-
const ValueDecl *findRequirementDeclWithDocComment(const ValueDecl *VD,
439-
bool AllowSerialized = false) {
435+
const ValueDecl *findRequirementDeclWithDocComment(const ValueDecl *VD) {
440436
std::queue<const ValueDecl *> requirements;
441437
while (true) {
442438
for (auto *req : VD->getSatisfiedProtocolRequirements()) {
443-
if (!req->getRawComment(AllowSerialized).isEmpty())
439+
if (!req->getRawComment().isEmpty())
444440
return req;
445441
else
446442
requirements.push(req);
@@ -453,38 +449,36 @@ const ValueDecl *findRequirementDeclWithDocComment(const ValueDecl *VD,
453449
}
454450
} // namespace
455451

456-
const Decl *swift::getDocCommentProvidingDecl(const Decl *D,
457-
bool AllowSerialized) {
452+
const Decl *swift::getDocCommentProvidingDecl(const Decl *D) {
458453
if (!D->canHaveComment())
459454
return nullptr;
460455

461-
if (!D->getRawComment(AllowSerialized).isEmpty())
456+
if (!D->getRawComment().isEmpty())
462457
return D;
463458

464459
auto *VD = dyn_cast<ValueDecl>(D);
465460
if (!VD)
466461
return nullptr;
467462

468-
if (auto *overridden = findOverriddenDeclWithDocComment(VD, AllowSerialized))
463+
if (auto *overridden = findOverriddenDeclWithDocComment(VD))
469464
return overridden;
470465

471-
if (auto *requirement = findDefaultProvidedDeclWithDocComment(VD, AllowSerialized))
466+
if (auto *requirement = findDefaultProvidedDeclWithDocComment(VD))
472467
return requirement;
473468

474-
if (auto *requirement = findRequirementDeclWithDocComment(VD, AllowSerialized))
469+
if (auto *requirement = findRequirementDeclWithDocComment(VD))
475470
return requirement;
476471

477472
return nullptr;
478473
}
479474

480-
DocComment *
481-
swift::getCascadingDocComment(swift::markup::MarkupContext &MC, const Decl *D,
482-
bool AllowSerialized) {
483-
auto *docD = getDocCommentProvidingDecl(D, AllowSerialized);
475+
DocComment *swift::getCascadingDocComment(swift::markup::MarkupContext &MC,
476+
const Decl *D) {
477+
auto *docD = getDocCommentProvidingDecl(D);
484478
if (!docD)
485479
return nullptr;
486480

487-
auto *doc = getSingleDocComment(MC, docD, AllowSerialized);
481+
auto *doc = getSingleDocComment(MC, docD);
488482
assert(doc && "getDocCommentProvidingDecl() returned decl with no comment");
489483

490484
// If the doc-comment is inherited from other decl, add a note about it.

‎lib/AST/Module.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,7 @@ SourceFile::getExternalRawLocsForDecl(const Decl *D) const {
14401440
Result.SourceFilePath = SM.getIdentifierForBuffer(BufferID);
14411441
setLoc(Result.Loc, MainLoc);
14421442
if (!InGeneratedBuffer) {
1443-
for (const auto &SRC : D->getRawComment(/*SerializedOK=*/false).Comments) {
1443+
for (const auto &SRC : D->getRawComment().Comments) {
14441444
Result.DocRanges.emplace_back(ExternalSourceLocs::RawLoc(),
14451445
SRC.Range.getByteLength());
14461446
setLoc(Result.DocRanges.back().first, SRC.Range.getStart());

‎lib/AST/RawComment.cpp

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,14 @@ static RawComment toRawComment(ASTContext &Context, CharSourceRange Range) {
130130
return Result;
131131
}
132132

133-
RawComment Decl::getRawComment(bool SerializedOK) const {
133+
RawComment Decl::getRawComment() const {
134134
if (!this->canHaveComment())
135135
return RawComment();
136136

137137
// Check the cache in ASTContext.
138138
auto &Context = getASTContext();
139139
if (Optional<std::pair<RawComment, bool>> RC = Context.getRawComment(this)) {
140-
auto P = RC.value();
141-
if (!SerializedOK || P.second)
142-
return P.first;
140+
return RC.value().first;
143141
}
144142

145143
// Check the declaration itself.
@@ -157,26 +155,24 @@ RawComment Decl::getRawComment(bool SerializedOK) const {
157155

158156
switch (Unit->getKind()) {
159157
case FileUnitKind::SerializedAST: {
160-
if (SerializedOK) {
161-
auto *CachedLocs = getSerializedLocs();
162-
if (!CachedLocs->DocRanges.empty()) {
163-
SmallVector<SingleRawComment, 4> SRCs;
164-
for (const auto &Range : CachedLocs->DocRanges) {
165-
if (Range.isValid()) {
166-
SRCs.push_back({Range, Context.SourceMgr});
167-
} else {
168-
// if we've run into an invalid range, don't bother trying to load
169-
// any of the other comments
170-
SRCs.clear();
171-
break;
172-
}
158+
auto *CachedLocs = getSerializedLocs();
159+
if (!CachedLocs->DocRanges.empty()) {
160+
SmallVector<SingleRawComment, 4> SRCs;
161+
for (const auto &Range : CachedLocs->DocRanges) {
162+
if (Range.isValid()) {
163+
SRCs.push_back({Range, Context.SourceMgr});
164+
} else {
165+
// if we've run into an invalid range, don't bother trying to load
166+
// any of the other comments
167+
SRCs.clear();
168+
break;
173169
}
170+
}
174171

175-
if (!SRCs.empty()) {
176-
auto RC = RawComment(Context.AllocateCopy(llvm::makeArrayRef(SRCs)));
177-
Context.setRawComment(this, RC, true);
178-
return RC;
179-
}
172+
if (!SRCs.empty()) {
173+
auto RC = RawComment(Context.AllocateCopy(llvm::makeArrayRef(SRCs)));
174+
Context.setRawComment(this, RC, true);
175+
return RC;
180176
}
181177
}
182178

‎lib/IDE/IDETypeChecking.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,9 @@ struct SynthesizedExtensionAnalyzer::Implementation {
294294
ExtensionDecl *EnablingExt, NormalProtocolConformance *Conf) {
295295
SynthesizedExtensionInfo Result(IsSynthesized, EnablingExt);
296296
ExtensionMergeInfo MergeInfo;
297-
MergeInfo.Unmergable = !Ext->getRawComment(/*SerializedOK=*/false).isEmpty() || // With comments
298-
Ext->getAttrs().hasAttribute<AvailableAttr>(); // With @available
297+
MergeInfo.Unmergable =
298+
!Ext->getRawComment().isEmpty() || // With comments
299+
Ext->getAttrs().hasAttribute<AvailableAttr>(); // With @available
299300
MergeInfo.InheritsCount = countInherits(Ext);
300301

301302
// There's (up to) two extensions here: the extension with the items that we

‎lib/IDE/ModuleInterfacePrinting.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -829,7 +829,7 @@ static SourceLoc getDeclStartPosition(SourceFile &File) {
829829
for (auto D : File.getTopLevelDecls()) {
830830
if (tryUpdateStart(D->getStartLoc())) {
831831
tryUpdateStart(D->getAttrs().getStartLoc());
832-
auto RawComment = D->getRawComment(/*SerializedOK=*/false);
832+
auto RawComment = D->getRawComment();
833833
if (!RawComment.isEmpty())
834834
tryUpdateStart(RawComment.Comments.front().Range.getStart());
835835
}

‎lib/IDE/SwiftSourceDocInfo.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ bool NameMatcher::handleCustomAttrs(Decl *D) {
198198

199199
ASTWalker::PreWalkAction NameMatcher::walkToDeclPre(Decl *D) {
200200
// Handle occurrences in any preceding doc comments
201-
RawComment R = D->getRawComment(/*SerializedOK=*/false);
201+
RawComment R = D->getRawComment();
202202
if (!R.isEmpty()) {
203203
for(SingleRawComment C: R.Comments) {
204204
while(!shouldSkip(C.Range))

‎lib/IDE/SyntaxModel.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ CharSourceRange innerCharSourceRangeFromSourceRange(const SourceManager &SM,
514514
static void setDecl(SyntaxStructureNode &N, Decl *D) {
515515
N.Dcl = D;
516516
N.Attrs = D->getOriginalAttrs();
517-
N.DocRange = D->getRawComment(/*SerializedOK=*/false).getCharSourceRange();
517+
N.DocRange = D->getRawComment().getCharSourceRange();
518518
}
519519

520520
} // anonymous namespace

‎lib/SymbolGraphGen/Symbol.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,8 @@ void Symbol::serializeRange(size_t InitialIndentation,
210210

211211
const ValueDecl *Symbol::getDeclInheritingDocs() const {
212212
// get the decl that would provide docs for this symbol
213-
const auto *DocCommentProvidingDecl = dyn_cast_or_null<ValueDecl>(
214-
getDocCommentProvidingDecl(D, /*AllowSerialized=*/true));
213+
const auto *DocCommentProvidingDecl =
214+
dyn_cast_or_null<ValueDecl>(getDocCommentProvidingDecl(D));
215215

216216
// if the decl is the same as the one for this symbol, we're not
217217
// inheriting docs, so return null. however, if this symbol is
@@ -356,13 +356,13 @@ void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
356356

357357
const auto *DocCommentProvidingDecl = D;
358358
if (!Graph->Walker.Options.SkipInheritedDocs) {
359-
DocCommentProvidingDecl = dyn_cast_or_null<ValueDecl>(
360-
getDocCommentProvidingDecl(D, /*AllowSerialized=*/true));
359+
DocCommentProvidingDecl =
360+
dyn_cast_or_null<ValueDecl>(getDocCommentProvidingDecl(D));
361361
if (!DocCommentProvidingDecl) {
362362
DocCommentProvidingDecl = D;
363363
}
364364
}
365-
auto RC = DocCommentProvidingDecl->getRawComment(/*SerializedOK=*/true);
365+
auto RC = DocCommentProvidingDecl->getRawComment();
366366
if (RC.isEmpty()) {
367367
return;
368368
}

‎lib/SymbolGraphGen/SymbolGraph.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -705,8 +705,7 @@ bool SymbolGraph::isImplicitlyPrivate(const Decl *D,
705705
// If we've been asked to skip protocol implementations, filter them out here.
706706
if (Walker.Options.SkipProtocolImplementations && getProtocolRequirement(VD)) {
707707
// Allow them to stay if they have their own doc comment
708-
const auto *DocCommentProvidingDecl =
709-
getDocCommentProvidingDecl(VD, /*AllowSerialized=*/true);
708+
const auto *DocCommentProvidingDecl = getDocCommentProvidingDecl(VD);
710709
if (DocCommentProvidingDecl != VD)
711710
return true;
712711
}

0 commit comments

Comments
 (0)