Skip to content

Commit f4ae514

Browse files
authored
Merge pull request #33475 from rintaro/ide-completion-inactiveifconfig-rdar67027408
[CodeCompletion] Parse #if block containing CC token as active
2 parents 0e6cf6d + 6cfdaf6 commit f4ae514

File tree

7 files changed

+163
-47
lines changed

7 files changed

+163
-47
lines changed

include/swift/AST/NameLookup.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -741,8 +741,6 @@ class ASTScope {
741741
return Mem;
742742
}
743743

744-
static bool areInactiveIfConfigClausesSupported();
745-
746744
private:
747745
static ast_scope::ASTSourceFileScope *createScopeTree(SourceFile *);
748746

lib/AST/ASTScopeCreation.cpp

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -479,23 +479,6 @@ class ScopeCreator final {
479479
}
480480

481481
public:
482-
/// When ASTScopes are enabled for code completion,
483-
/// IfConfigs will pose a challenge because we may need to field lookups into
484-
/// the inactive clauses, but the AST contains redundancy: the active clause's
485-
/// elements are present in the members or elements of an IterableTypeDecl or
486-
/// BraceStmt alongside of the IfConfigDecl. In addition there are two more
487-
/// complications:
488-
///
489-
/// 1. The active clause's elements may be nested inside an init self
490-
/// rebinding decl (as in StringObject.self).
491-
///
492-
/// 2. The active clause may be before or after the inactive ones
493-
///
494-
/// So, when encountering an IfConfigDecl, we will expand the inactive
495-
/// elements. Also, always sort members or elements so that the child scopes
496-
/// are in source order (Just one of several reasons we need to sort.)
497-
///
498-
static const bool includeInactiveIfConfigClauses = false;
499482

500483
private:
501484
static std::vector<ASTNode> expandIfConfigClauses(ArrayRef<ASTNode> input) {
@@ -526,9 +509,6 @@ class ScopeCreator final {
526509
if (isa<IfConfigDecl>(d))
527510
expandIfConfigClausesInto(expansion, {d}, true);
528511
}
529-
} else if (includeInactiveIfConfigClauses) {
530-
expandIfConfigClausesInto(expansion, clause.Elements,
531-
/*isInAnActiveNode=*/false);
532512
}
533513
}
534514
}
@@ -762,10 +742,6 @@ void ASTScope::
762742
impl->buildEnoughOfTreeForTopLevelExpressionsButDontRequestGenericsOrExtendedNominals();
763743
}
764744

765-
bool ASTScope::areInactiveIfConfigClausesSupported() {
766-
return ScopeCreator::includeInactiveIfConfigClauses;
767-
}
768-
769745
void ASTScope::expandFunctionBody(AbstractFunctionDecl *AFD) {
770746
auto *const SF = AFD->getParentSourceFile();
771747
SF->getScope().expandFunctionBodyImpl(AFD);
@@ -2083,10 +2059,8 @@ class LocalizableDeclContextCollector : public ASTWalker {
20832059
// catchForDebugging(D, "DictionaryBridging.swift", 694);
20842060
if (const auto *dc = dyn_cast<DeclContext>(D))
20852061
record(dc);
2086-
if (auto *icd = dyn_cast<IfConfigDecl>(D)) {
2087-
walkToClauses(icd);
2062+
if (isa<IfConfigDecl>(D))
20882063
return false;
2089-
}
20902064
if (auto *pd = dyn_cast<ParamDecl>(D))
20912065
record(pd->getDefaultArgumentInitContext());
20922066
else if (auto *pbd = dyn_cast<PatternBindingDecl>(D))
@@ -2104,18 +2078,6 @@ class LocalizableDeclContextCollector : public ASTWalker {
21042078
}
21052079

21062080
private:
2107-
void walkToClauses(IfConfigDecl *icd) {
2108-
for (auto &clause : icd->getClauses()) {
2109-
// Generate scopes for any closures in the condition
2110-
if (ScopeCreator::includeInactiveIfConfigClauses && clause.isActive) {
2111-
if (clause.Cond)
2112-
clause.Cond->walk(*this);
2113-
for (auto n : clause.Elements)
2114-
n.walk(*this);
2115-
}
2116-
}
2117-
}
2118-
21192081
void recordInitializers(PatternBindingDecl *pbd) {
21202082
for (auto idx : range(pbd->getNumPatternEntries()))
21212083
record(pbd->getInitContext(idx));

lib/IDE/CompletionInstance.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@ template <typename Range> Decl *getElementAt(const Range &Decls, unsigned N) {
8585
/// Find the equivalent \c DeclContext with \p DC from \p SF AST.
8686
/// This assumes the AST which contains \p DC has exact the same structure with
8787
/// \p SF.
88-
/// FIXME: This doesn't support IfConfigDecl blocks. If \p DC is in an inactive
89-
/// config block, this function returns \c false.
9088
static DeclContext *getEquivalentDeclContextFromSourceFile(DeclContext *DC,
9189
SourceFile *SF) {
9290
PrettyStackTraceDeclContext trace("getting equivalent decl context for", DC);
@@ -129,7 +127,6 @@ static DeclContext *getEquivalentDeclContextFromSourceFile(DeclContext *DC,
129127
}
130128

131129
// Not found in the decl context tree.
132-
// FIXME: Probably DC is in an inactive #if block.
133130
if (N == ~0U) {
134131
return nullptr;
135132
}

lib/Parse/ParseIfConfig.cpp

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,17 +603,42 @@ static Expr *findAnyLikelySimulatorEnvironmentTest(Expr *Condition) {
603603
/// Delegate callback function to parse elements in the blocks.
604604
ParserResult<IfConfigDecl> Parser::parseIfConfig(
605605
llvm::function_ref<void(SmallVectorImpl<ASTNode> &, bool)> parseElements) {
606+
assert(Tok.is(tok::pound_if));
606607
SyntaxParsingContext IfConfigCtx(SyntaxContext, SyntaxKind::IfConfigDecl);
607608

608609
SmallVector<IfConfigClause, 4> Clauses;
609610
Parser::StructureMarkerRAII ParsingDecl(
610611
*this, Tok.getLoc(), Parser::StructureMarkerKind::IfConfig);
611612

613+
// Find the region containing code completion token.
614+
SourceLoc codeCompletionClauseLoc;
615+
if (SourceMgr.hasCodeCompletionBuffer() &&
616+
SourceMgr.getCodeCompletionBufferID() == L->getBufferID() &&
617+
SourceMgr.isBeforeInBuffer(Tok.getLoc(),
618+
SourceMgr.getCodeCompletionLoc())) {
619+
llvm::SaveAndRestore<Optional<llvm::MD5>> H(CurrentTokenHash, None);
620+
BacktrackingScope backtrack(*this);
621+
do {
622+
auto startLoc = Tok.getLoc();
623+
consumeToken();
624+
skipUntilConditionalBlockClose();
625+
auto endLoc = PreviousLoc;
626+
if (SourceMgr.rangeContainsTokenLoc(SourceRange(startLoc, endLoc),
627+
SourceMgr.getCodeCompletionLoc())){
628+
codeCompletionClauseLoc = startLoc;
629+
break;
630+
}
631+
} while (Tok.isNot(tok::pound_endif, tok::eof));
632+
}
633+
612634
bool shouldEvaluate =
613635
// Don't evaluate if it's in '-parse' mode, etc.
614636
shouldEvaluatePoundIfDecls() &&
615637
// If it's in inactive #if ... #endif block, there's no point to do it.
616-
!getScopeInfo().isInactiveConfigBlock();
638+
!getScopeInfo().isInactiveConfigBlock() &&
639+
// If this directive contains code completion location, 'isActive' is
640+
// determined solely by which block has the completion token.
641+
!codeCompletionClauseLoc.isValid();
617642

618643
bool foundActive = false;
619644
bool isVersionCondition = false;
@@ -658,6 +683,10 @@ ParserResult<IfConfigDecl> Parser::parseIfConfig(
658683
}
659684
}
660685

686+
// Treat the region containing code completion token as "active".
687+
if (codeCompletionClauseLoc.isValid() && !foundActive)
688+
isActive = (ClauseLoc == codeCompletionClauseLoc);
689+
661690
foundActive |= isActive;
662691

663692
if (!Tok.isAtStartOfLine() && Tok.isNot(tok::eof)) {

lib/Parse/Parser.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -695,8 +695,9 @@ void Parser::skipSingle() {
695695
void Parser::skipUntil(tok T1, tok T2) {
696696
// tok::NUM_TOKENS is a sentinel that means "don't skip".
697697
if (T1 == tok::NUM_TOKENS && T2 == tok::NUM_TOKENS) return;
698-
699-
while (Tok.isNot(T1, T2, tok::eof, tok::pound_endif, tok::code_complete))
698+
699+
while (Tok.isNot(T1, T2, tok::eof, tok::pound_endif, tok::pound_else,
700+
tok::pound_elseif))
700701
skipSingle();
701702
}
702703

test/IDE/complete_in_ifconfig.swift

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t
3+
4+
struct MyStruct {
5+
init() {}
6+
var value: Int
7+
}
8+
9+
// MEMBER_MyStruct: Begin completions, 2 items
10+
// MEMBER_MyStruct-DAG: Keyword[self]/CurrNominal: self[#MyStruct#];
11+
// MEMBER_MyStruct-DAG: Decl[InstanceVar]/CurrNominal: value[#Int#];
12+
// MEMBER_MyStruct: End completions
13+
14+
#if true
15+
let toplevelActive = MyStruct()
16+
_ = toplevelActive.#^MEMBER_TOPLEVEL_ACTIVE?check=MEMBER_MyStruct^#
17+
#else
18+
let toplevelInactive = MyStruct()
19+
_ = toplevelInactive.#^MEMBER_TOPLEVEL_INACTIVE?check=MEMBER_MyStruct^#
20+
#endif
21+
22+
func foo() {
23+
#if true
24+
let infuncActive = MyStruct()
25+
_ = infuncActive.#^MEMBER_INFUNC_ACTIVE?check=MEMBER_MyStruct^#
26+
#else
27+
let infuncInactive = MyStruct()
28+
_ = infuncInactive.#^MEMBER_INFUNC_INACTIVE?check=MEMBER_MyStruct^#
29+
#endif
30+
}
31+
32+
protocol TestP {
33+
func foo()
34+
func bar()
35+
}
36+
struct TestStruct: TestP {
37+
#if true
38+
func foo() {}
39+
func #^OVERRIDE_ACTIVE^#
40+
// OVERRIDE_ACTIVE: Begin completions, 1 items
41+
// OVERRIDE_ACTIVE-DAG: Decl[InstanceMethod]/Super: bar() {|};
42+
// OVERRIDE_ACTIVE: End completions
43+
#else
44+
func bar() {}
45+
func #^OVERRIDE_INACTIVE^#
46+
// OVERRIDE_INACTIVE: Begin completions, 1 items
47+
// OVERRIDE_INACTIVE-DAG: Decl[InstanceMethod]/Super: foo() {|};
48+
// OVERRIDE_INACTIVE: End completions
49+
#endif
50+
}
51+
52+
struct TestStruct2 {
53+
#if true
54+
func activeFunc() {}
55+
func test() {
56+
self.#^SELF_ACTIVE^#
57+
}
58+
// SELF_ACTIVE: Begin completions, 3 items
59+
// SELF_ACTIVE-DAG: Keyword[self]/CurrNominal: self[#TestStruct2#];
60+
// SELF_ACTIVE-DAG: Decl[InstanceMethod]/CurrNominal: activeFunc()[#Void#];
61+
// SELF_ACTIVE-DAG: Decl[InstanceMethod]/CurrNominal: test()[#Void#];
62+
// SELF_ACTIVE: End completions
63+
#else
64+
func inactiveFunc() {}
65+
func test() {
66+
self.#^SELF_INACTIVE^#
67+
}
68+
// SELF_INACTIVE: Begin completions, 3 items
69+
// SELF_INACTIVE-DAG: Keyword[self]/CurrNominal: self[#TestStruct2#];
70+
// SELF_INACTIVE-DAG: Decl[InstanceMethod]/CurrNominal: inactiveFunc()[#Void#];
71+
// SELF_INACTIVE-DAG: Decl[InstanceMethod]/CurrNominal: test()[#Void#];
72+
// SELF_INACTIVE: End completions
73+
#endif
74+
}
75+
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
struct MyStruct {
2+
init() {}
3+
var value: Int = 1
4+
}
5+
6+
func foo(arg: MyStruct) {
7+
#if true
8+
_ = arg./*8:11*/
9+
#else
10+
_ = arg./*10:11*/
11+
#endif
12+
}
13+
14+
struct TestStruct {
15+
#if true
16+
func testActive(arg: MyStruct) {
17+
_ = arg./*17:13*/
18+
}
19+
#else
20+
func testInactive(arg: MyStruct) {
21+
_ = arg./*21:13*/
22+
}
23+
#endif
24+
}
25+
26+
// Test that (1) fast completion happens even in inactive #if blocks, and
27+
// (2) #if in toplevel decls invalidate cached ASTContext
28+
29+
// RUN: %sourcekitd-test \
30+
// RUN: -req=complete -pos=8:11 %s -- %s -parse-as-library == \
31+
// RUN: -req=complete -pos=10:11 %s -- %s -parse-as-library == \
32+
// RUN: -req=complete -pos=17:13 %s -- %s -parse-as-library == \
33+
// RUN: -req=complete -pos=21:13 %s -- %s -parse-as-library \
34+
// RUN: | %FileCheck %s --check-prefix=RESULT
35+
36+
// RESULT-LABEL: key.results: [
37+
// RESULT-DAG: key.description: "value"
38+
// RESULT: ]
39+
// RESULT-NOT: key.reusingastcontext: 1
40+
41+
// RESULT-LABEL: key.results: [
42+
// RESULT-DAG: key.description: "value"
43+
// RESULT: ]
44+
// RESULT: key.reusingastcontext: 1
45+
46+
// RESULT-LABEL: key.results: [
47+
// RESULT-DAG: key.description: "value"
48+
// RESULT: ]
49+
// RESULT: key.reusingastcontext: 1
50+
51+
// RESULT-LABEL: key.results: [
52+
// RESULT-DAG: key.description: "value"
53+
// RESULT: ]
54+
// RESULT-NOT: key.reusingastcontext: 1

0 commit comments

Comments
 (0)