Skip to content

Commit dc1bc82

Browse files
author
Harlan
authored
[InterfaceGen] Remove #ifs from default arguments (#19075)
* [InterfaceGen] Remove #ifs from default args This patch removes all #if configs form the bodies of default arguments, which can contain multiline closures, while preserving the bodies of the clauses that are active. This code is generalized and should "just work" for inlinable function bodies, which will come in a later patch. * Address review comments * Fix and test CharSourceRange.overlaps * Fix CharSourceRange::print to respect half-open ranges
1 parent 3e260c9 commit dc1bc82

17 files changed

+392
-56
lines changed

include/swift/AST/Decl.h

+22-7
Original file line numberDiff line numberDiff line change
@@ -4770,14 +4770,29 @@ class ParamDecl : public VarDecl {
47704770

47714771
void setDefaultArgumentInitContext(Initializer *initContext);
47724772

4773-
/// Returns a saved string representation of the parameter's default value.
4774-
///
4775-
/// This should only be called if the default value expression is absent or
4776-
/// doesn't have a valid source range; otherwise, clients should extract the
4777-
/// source text from that range.
4778-
///
4773+
/// Extracts the text of the default argument attached to the provided
4774+
/// ParamDecl, removing all inactive #if clauses and providing only the
4775+
/// text of active #if clauses.
4776+
///
4777+
/// For example, the default argument:
4778+
/// ```
4779+
/// {
4780+
/// #if false
4781+
/// print("false")
4782+
/// #else
4783+
/// print("true")
4784+
/// #endif
4785+
/// }
4786+
/// ```
4787+
/// will return
4788+
/// ```
4789+
/// {
4790+
/// print("true")
4791+
/// }
4792+
/// ```
47794793
/// \sa getDefaultValue
4780-
StringRef getDefaultValueStringRepresentation() const;
4794+
StringRef getDefaultValueStringRepresentation(
4795+
SmallVectorImpl<char> &scratch) const;
47814796

47824797
void setDefaultValueStringRepresentation(StringRef stringRepresentation);
47834798

include/swift/AST/PrintOptions.h

-5
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,6 @@ struct PrintOptions {
147147
/// \brief Whether to print variable initializers.
148148
bool VarInitializers = false;
149149

150-
/// \brief Whether to print a placeholder for default parameters.
151-
bool PrintDefaultParameterPlaceholder = true;
152-
153150
/// \brief Whether to print enum raw value expressions.
154151
bool EnumRawValues = false;
155152

@@ -381,7 +378,6 @@ struct PrintOptions {
381378
PrintOptions result;
382379
result.TypeDefinitions = true;
383380
result.VarInitializers = true;
384-
result.PrintDefaultParameterPlaceholder = true;
385381
result.PrintDocumentationComments = true;
386382
result.PrintRegularClangComments = true;
387383
result.PrintLongAttrsOnSeparateLines = true;
@@ -509,7 +505,6 @@ struct PrintOptions {
509505
static PrintOptions printQuickHelpDeclaration() {
510506
PrintOptions PO;
511507
PO.EnumRawValues = true;
512-
PO.PrintDefaultParameterPlaceholder = true;
513508
PO.PrintImplicitAttrs = false;
514509
PO.PrintFunctionRepresentationAttrs = false;
515510
PO.PrintDocumentationComments = false;

include/swift/Basic/SourceLoc.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ class CharSourceRange {
184184
}
185185

186186
bool overlaps(CharSourceRange Other) const {
187-
return contains(Other.getStart()) || contains(Other.getEnd());
187+
if (getByteLength() == 0 || Other.getByteLength() == 0) return false;
188+
return contains(Other.getStart()) || Other.contains(getStart());
188189
}
189190

190191
StringRef str() const {

lib/AST/ASTPrinter.cpp

+17-21
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ PrintOptions PrintOptions::printTextualInterfaceFile() {
111111
result.PrintAccess = true;
112112

113113
result.ExcludeAttrList = {DAK_ImplicitlyUnwrappedOptional, DAK_AccessControl};
114-
result.PrintDefaultParameterPlaceholder = false;
115114

116115
return result;
117116
}
@@ -2365,27 +2364,24 @@ void PrintAST::printOneParameter(const ParamDecl *param,
23652364
Printer << "...";
23662365

23672366
if (param->isDefaultArgument()) {
2368-
StringRef defaultArgStr = param->getDefaultValueStringRepresentation();
2367+
SmallString<128> scratch;
2368+
auto defaultArgStr = param->getDefaultValueStringRepresentation(scratch);
23692369

2370-
if (defaultArgStr.empty()) {
2371-
if (Options.PrintDefaultParameterPlaceholder)
2372-
Printer << " = " << tok::kw_default;
2373-
} else {
2374-
Printer << " = ";
2375-
2376-
switch (param->getDefaultArgumentKind()) {
2377-
case DefaultArgumentKind::File:
2378-
case DefaultArgumentKind::Line:
2379-
case DefaultArgumentKind::Column:
2380-
case DefaultArgumentKind::Function:
2381-
case DefaultArgumentKind::DSOHandle:
2382-
case DefaultArgumentKind::NilLiteral:
2383-
Printer.printKeyword(defaultArgStr);
2384-
break;
2385-
default:
2386-
Printer << defaultArgStr;
2387-
break;
2388-
}
2370+
assert(!defaultArgStr.empty() && "empty default argument?");
2371+
Printer << " = ";
2372+
2373+
switch (param->getDefaultArgumentKind()) {
2374+
case DefaultArgumentKind::File:
2375+
case DefaultArgumentKind::Line:
2376+
case DefaultArgumentKind::Column:
2377+
case DefaultArgumentKind::Function:
2378+
case DefaultArgumentKind::DSOHandle:
2379+
case DefaultArgumentKind::NilLiteral:
2380+
Printer.printKeyword(defaultArgStr);
2381+
break;
2382+
default:
2383+
Printer << defaultArgStr;
2384+
break;
23892385
}
23902386
}
23912387
}

lib/AST/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ add_swift_library(swiftAST STATIC
3434
GenericSignature.cpp
3535
GenericSignatureBuilder.cpp
3636
Identifier.cpp
37+
InlinableText.cpp
3738
LayoutConstraint.cpp
3839
LookupVisibleDecls.cpp
3940
Module.cpp

lib/AST/Decl.cpp

+12-3
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
#include "clang/AST/Attr.h"
5656
#include "clang/AST/DeclObjC.h"
5757

58+
#include "InlinableText.h"
5859
#include <algorithm>
5960

6061
using namespace swift;
@@ -4829,14 +4830,22 @@ void ParamDecl::setDefaultArgumentInitContext(Initializer *initContext) {
48294830
DefaultValueAndIsVariadic.getPointer()->InitContext = initContext;
48304831
}
48314832

4832-
StringRef ParamDecl::getDefaultValueStringRepresentation() const {
4833+
StringRef
4834+
ParamDecl::getDefaultValueStringRepresentation(
4835+
SmallVectorImpl<char> &scratch) const {
48334836
switch (getDefaultArgumentKind()) {
48344837
case DefaultArgumentKind::None:
48354838
llvm_unreachable("called on a ParamDecl with no default value");
4836-
case DefaultArgumentKind::Normal:
4839+
case DefaultArgumentKind::Normal: {
48374840
assert(DefaultValueAndIsVariadic.getPointer() &&
48384841
"default value not provided yet");
4839-
return DefaultValueAndIsVariadic.getPointer()->StringRepresentation;
4842+
auto existing =
4843+
DefaultValueAndIsVariadic.getPointer()->StringRepresentation;
4844+
if (!existing.empty())
4845+
return existing;
4846+
return extractInlinableText(getASTContext().SourceMgr, getDefaultValue(),
4847+
scratch);
4848+
}
48404849
case DefaultArgumentKind::Inherited:
48414850
// FIXME: This needs /some/ kind of textual representation, but this isn't
48424851
// a great one.

lib/AST/InlinableText.cpp

+161
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
//===---- InlinableText.cpp - Extract inlinable source text -----*- C++ -*-===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
#include "InlinableText.h"
13+
#include "swift/AST/ASTContext.h"
14+
#include "swift/AST/ASTWalker.h"
15+
#include "swift/AST/ASTNode.h"
16+
#include "swift/AST/Decl.h"
17+
#include "swift/Parse/Lexer.h"
18+
19+
#include "llvm/ADT/SmallVector.h"
20+
#include "llvm/ADT/SmallString.h"
21+
22+
using namespace swift;
23+
24+
/// Gets the last token that exists inside this IfConfigClause, ignoring
25+
/// hoisted elements.
26+
///
27+
/// If the clause is the last element, this returns the beginning of the line
28+
/// before the parent IfConfigDecl's #endif token. Otherwise, it's the beginning
29+
/// of the line before the next clause's #else or #elseif token.
30+
static SourceLoc
31+
getEffectiveEndLoc(SourceManager &sourceMgr, const IfConfigClause *clause,
32+
const IfConfigDecl *decl) {
33+
auto clauses = decl->getClauses();
34+
if (clause == &clauses.back())
35+
return Lexer::getLocForStartOfLine(sourceMgr, decl->getEndLoc());
36+
37+
assert(clause >= clauses.begin() && clause < clauses.end() &&
38+
"clauses must be contiguous");
39+
40+
auto *nextClause = clause + 1;
41+
return Lexer::getLocForStartOfLine(sourceMgr, nextClause->Loc);
42+
}
43+
44+
namespace {
45+
/// A walker that searches through #if declarations, finding all text that does
46+
/// not contribute to the final evaluated AST.
47+
///
48+
/// For example, in the following code:
49+
/// ```
50+
/// #if true
51+
/// print("true")
52+
/// #else
53+
/// print("false")
54+
/// #endif
55+
/// ```
56+
/// ExtractInactiveRanges will return the ranges (with leading newlines) of:
57+
/// ```
58+
/// #if true
59+
/// #else
60+
/// print("false")
61+
/// #endif
62+
/// ```
63+
/// Leaving behind just 'print("true")'s range.
64+
struct ExtractInactiveRanges : public ASTWalker {
65+
SmallVector<CharSourceRange, 4> ranges;
66+
SourceManager &sourceMgr;
67+
68+
explicit ExtractInactiveRanges(SourceManager &sourceMgr)
69+
: sourceMgr(sourceMgr) {}
70+
71+
/// Adds the two SourceLocs as a CharSourceRange to the set of ignored
72+
/// ranges.
73+
/// \note: This assumes each of these locs is a character location, not a
74+
/// token location.
75+
void addRange(SourceLoc start, SourceLoc end) {
76+
auto charRange = CharSourceRange(sourceMgr, start, end);
77+
ranges.push_back(charRange);
78+
}
79+
80+
bool walkToDeclPre(Decl *d) {
81+
auto icd = dyn_cast<IfConfigDecl>(d);
82+
if (!icd) return true;
83+
84+
auto start = Lexer::getLocForStartOfLine(sourceMgr, icd->getStartLoc());
85+
auto end = Lexer::getLocForEndOfLine(sourceMgr, icd->getEndLoc());
86+
87+
auto clause = icd->getActiveClause();
88+
89+
// If there's no active clause, add the entire #if...#endif block.
90+
if (!clause) {
91+
addRange(start, end);
92+
return false;
93+
}
94+
95+
// Ignore range from beginning of '#if' to the beginning of the elements
96+
// of this clause.
97+
addRange(start, Lexer::getLocForEndOfLine(sourceMgr, clause->Loc));
98+
99+
// Ignore range from effective end of the elements of this clause to the
100+
// end of the '#endif'
101+
addRange(getEffectiveEndLoc(sourceMgr, clause, icd), end);
102+
103+
// Walk into direct children of this node that are IfConfigDecls, because
104+
// the standard walker won't walk into them.
105+
for (auto &elt : clause->Elements)
106+
if (elt.isDecl(DeclKind::IfConfig))
107+
elt.get<Decl *>()->walk(*this);
108+
109+
return false;
110+
}
111+
112+
/// Gets the ignored ranges in source order.
113+
ArrayRef<CharSourceRange> getSortedRanges() {
114+
std::sort(ranges.begin(), ranges.end(),
115+
[&](CharSourceRange r1, CharSourceRange r2) {
116+
assert(!r1.overlaps(r2) && "no overlapping ranges");
117+
return sourceMgr.isBeforeInBuffer(r1.getStart(), r2.getStart());
118+
});
119+
return ranges;
120+
}
121+
};
122+
} // end anonymous namespace
123+
124+
StringRef swift::extractInlinableText(SourceManager &sourceMgr, ASTNode node,
125+
SmallVectorImpl<char> &scratch) {
126+
// Extract inactive ranges from the text of the node.
127+
ExtractInactiveRanges extractor(sourceMgr);
128+
node.walk(extractor);
129+
130+
// If there were no inactive ranges, then there were no #if configs.
131+
// Return an unowned buffer directly into the source file.
132+
if (extractor.ranges.empty()) {
133+
auto range =
134+
Lexer::getCharSourceRangeFromSourceRange(
135+
sourceMgr, node.getSourceRange());
136+
return sourceMgr.extractText(range);
137+
}
138+
139+
// Begin piecing together active code ranges.
140+
141+
// Get the full start and end of the provided node, as character locations.
142+
SourceLoc start = node.getStartLoc();
143+
SourceLoc end = Lexer::getLocForEndOfToken(sourceMgr, node.getEndLoc());
144+
for (auto &range : extractor.getSortedRanges()) {
145+
// Add the text from the current 'start' to this ignored range's start.
146+
auto charRange = CharSourceRange(sourceMgr, start, range.getStart());
147+
auto chunk = sourceMgr.extractText(charRange);
148+
scratch.append(chunk.begin(), chunk.end());
149+
150+
// Set 'start' to the end of this range, effectively skipping it.
151+
start = range.getEnd();
152+
}
153+
154+
// If there's leftover unignored text, add it.
155+
if (start != end) {
156+
auto range = CharSourceRange(sourceMgr, start, end);
157+
auto chunk = sourceMgr.extractText(range);
158+
scratch.append(chunk.begin(), chunk.end());
159+
}
160+
return { scratch.data(), scratch.size() };
161+
}

lib/AST/InlinableText.h

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//===------ InlinableText.h - Extract inlinable source text -----*- C++ -*-===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#ifndef SWIFT_AST_INLINABLETEXT_H
14+
#define SWIFT_AST_INLINABLETEXT_H
15+
16+
#include "swift/AST/ASTNode.h"
17+
#include "swift/Basic/LLVM.h"
18+
#include "llvm/ADT/StringRef.h"
19+
#include "llvm/ADT/SmallVector.h"
20+
21+
namespace swift {
22+
class SourceManager;
23+
24+
/// Extracts the text of this ASTNode from the source buffer, ignoring
25+
/// all #if declarations and clauses except the elements that are active.
26+
StringRef extractInlinableText(SourceManager &sourceMgr, ASTNode node,
27+
SmallVectorImpl<char> &scratch);
28+
29+
} // end namespace swift
30+
31+
#endif // defined(SWIFT_AST_INLINABLETEXT_H)

lib/Basic/SourceLoc.cpp

+1-4
Original file line numberDiff line numberDiff line change
@@ -302,10 +302,7 @@ void CharSourceRange::print(raw_ostream &OS, const SourceManager &SM,
302302
return;
303303

304304
if (PrintText) {
305-
OS << " RangeText=\""
306-
<< StringRef(Start.Value.getPointer(),
307-
getEnd().Value.getPointer() - Start.Value.getPointer() + 1)
308-
<< '"';
305+
OS << " RangeText=\"" << SM.extractText(*this) << '"';
309306
}
310307
}
311308

lib/IDE/CodeCompletion.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -4125,7 +4125,6 @@ class CompletionOverrideLookup : public swift::VisibleDeclConsumer {
41254125
PrintOptions Options;
41264126
if (auto transformType = CurrDeclContext->getDeclaredTypeInContext())
41274127
Options.setBaseType(transformType);
4128-
Options.PrintDefaultParameterPlaceholder = false;
41294128
Options.PrintImplicitAttrs = false;
41304129
Options.ExclusiveAttrList.push_back(TAK_escaping);
41314130
Options.PrintOverrideKeyword = false;
@@ -4222,7 +4221,6 @@ class CompletionOverrideLookup : public swift::VisibleDeclConsumer {
42224221
PrintOptions Options;
42234222
Options.PrintImplicitAttrs = false;
42244223
Options.SkipAttributes = true;
4225-
Options.PrintDefaultParameterPlaceholder = false;
42264224
CD->print(OS, Options);
42274225
}
42284226
Builder.addTextChunk(DeclStr);

lib/IDE/ModuleInterfacePrinting.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,6 @@ static void adjustPrintOptions(PrintOptions &AdjustedOptions) {
207207
// Print var declarations separately, one variable per decl.
208208
AdjustedOptions.ExplodePatternBindingDecls = true;
209209
AdjustedOptions.VarInitializers = false;
210-
211-
AdjustedOptions.PrintDefaultParameterPlaceholder = true;
212210
}
213211

214212
ArrayRef<StringRef>

0 commit comments

Comments
 (0)