Skip to content

Commit 07054af

Browse files
committed
Tie attributes to language features
The new `DECL_ATTR_FEATURE_REQUIREMENT` macro in DeclAttr.def can be used to declare that an attribute should only be available when a related language feature is enabled. Effects: • `#if hasAttribute(someAttr)` will return `false` unless the required feature is enabled. • Code completion will not include the attribute unless the required feature is enabled. • `TypeChecker::checkDeclAttributes()` diagnoses non-implicit uses of the attribute. Add this mechanism and use it to tie @abi to the ABIAttribute feature. Also design tests for it.
1 parent af0a7ab commit 07054af

File tree

11 files changed

+236
-14
lines changed

11 files changed

+236
-14
lines changed

include/swift/AST/Attr.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "swift/AST/StorageImpl.h"
3232
#include "swift/Basic/Debug.h"
3333
#include "swift/Basic/EnumTraits.h"
34+
#include "swift/Basic/Feature.h"
3435
#include "swift/Basic/InlineBitfield.h"
3536
#include "swift/Basic/Located.h"
3637
#include "swift/Basic/OptimizationMode.h"
@@ -493,6 +494,8 @@ class DeclAttribute : public AttributeBase {
493494
LLVM_READNONE
494495
static bool canAttributeAppearOnDeclKind(DeclAttrKind DAK, DeclKind DK);
495496

497+
static std::optional<Feature> getRequiredFeature(DeclAttrKind DK);
498+
496499
/// Returns the source name of the attribute, without the @ or any arguments.
497500
StringRef getAttrName() const;
498501

include/swift/AST/DeclAttr.def

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@
4242
DECL_ATTR_ALIAS(SPELLING, CLASS)
4343
#endif
4444

45+
// Diagnose any use of the attribute CLASS without FEATURE_NAME enabled,
46+
// and also enable other special behavior. If you use this for an experimental
47+
// feature, please add test cases to:
48+
//
49+
// * test/attr/feature_requirement.swift
50+
// * test/IDE/complete_decl_attribute_feature_requirement.swift
51+
#ifndef DECL_ATTR_FEATURE_REQUIREMENT
52+
#define DECL_ATTR_FEATURE_REQUIREMENT(CLASS, FEATURE_NAME)
53+
#endif
54+
4555
#ifndef LAST_DECL_ATTR
4656
#define LAST_DECL_ATTR(CLASS)
4757
#endif
@@ -516,6 +526,7 @@ DECL_ATTR(lifetime, Lifetime,
516526
DECL_ATTR(abi, ABI,
517527
OnAbstractFunction | OnVar /* will eventually add types */ | LongAttribute | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove,
518528
162)
529+
DECL_ATTR_FEATURE_REQUIREMENT(ABI, ABIAttribute)
519530

520531
LAST_DECL_ATTR(ABI)
521532

@@ -525,4 +536,5 @@ LAST_DECL_ATTR(ABI)
525536
#undef CONTEXTUAL_SIMPLE_DECL_ATTR
526537
#undef DECL_ATTR
527538
#undef CONTEXTUAL_DECL_ATTR
539+
#undef DECL_ATTR_FEATURE_REQUIREMENT
528540
#undef LAST_DECL_ATTR

include/swift/IDE/CompletionLookup.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
600600
SourceLoc CodeCompletionLoc);
601601

602602
static bool canUseAttributeOnDecl(DeclAttrKind DAK, bool IsInSil,
603-
bool IsConcurrencyEnabled,
603+
const LangOptions &langOpts,
604604
std::optional<DeclKind> DK, StringRef Name);
605605

606606
void getAttributeDeclCompletions(bool IsInSil, std::optional<DeclKind> DK);

lib/AST/Attr.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1646,6 +1646,18 @@ uint64_t DeclAttribute::getOptions(DeclAttrKind DK) {
16461646
llvm_unreachable("bad DeclAttrKind");
16471647
}
16481648

1649+
std::optional<Feature> DeclAttribute::getRequiredFeature(DeclAttrKind DK) {
1650+
switch (DK) {
1651+
#define DECL_ATTR_FEATURE_REQUIREMENT(CLASS, FEATURE_NAME) \
1652+
case DeclAttrKind::CLASS: \
1653+
return Feature::FEATURE_NAME;
1654+
#include "swift/AST/DeclAttr.def"
1655+
default:
1656+
return std::nullopt;
1657+
}
1658+
llvm_unreachable("bad DeclAttrKind");
1659+
}
1660+
16491661
StringRef DeclAttribute::getAttrName() const {
16501662
switch (getKind()) {
16511663
#define SIMPLE_DECL_ATTR(NAME, CLASS, ...) \
@@ -2896,6 +2908,10 @@ static bool hasDeclAttribute(const LangOptions &langOpts,
28962908
if (DeclAttribute::isConcurrencyOnly(*kind))
28972909
return false;
28982910

2911+
if (auto feature = DeclAttribute::getRequiredFeature(*kind))
2912+
if (!langOpts.hasFeature(*feature))
2913+
return false;
2914+
28992915
return true;
29002916
}
29012917

lib/IDE/CodeCompletion.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ static void addKeyword(CodeCompletionResultSink &Sink, StringRef Name,
686686
}
687687

688688
static void addDeclKeywords(CodeCompletionResultSink &Sink, DeclContext *DC,
689-
bool IsConcurrencyEnabled) {
689+
const LangOptions &langOpts) {
690690
auto isTypeDeclIntroducer = [](CodeCompletionKeywordKind Kind,
691691
std::optional<DeclAttrKind> DAK) -> bool {
692692
switch (Kind) {
@@ -799,10 +799,16 @@ static void addDeclKeywords(CodeCompletionResultSink &Sink, DeclContext *DC,
799799
return;
800800

801801
// Remove keywords only available when concurrency is enabled.
802-
if (DAK.has_value() && !IsConcurrencyEnabled &&
802+
if (DAK.has_value() && !langOpts.EnableExperimentalConcurrency &&
803803
DeclAttribute::isConcurrencyOnly(*DAK))
804804
return;
805805

806+
// Remove experimental keywords.
807+
if (DAK.has_value())
808+
if (auto feature = DeclAttribute::getRequiredFeature(*DAK))
809+
if (!langOpts.hasFeature(*feature))
810+
return;
811+
806812
CodeCompletionFlair flair = getFlair(Kind, DAK);
807813

808814
// Special case for 'actor'. Get the same flair with 'kw_class'.
@@ -1019,8 +1025,7 @@ void CodeCompletionCallbacksImpl::addKeywords(CodeCompletionResultSink &Sink,
10191025
LLVM_FALLTHROUGH;
10201026
}
10211027
case CompletionKind::StmtOrExpr:
1022-
addDeclKeywords(Sink, CurDeclContext,
1023-
Context.LangOpts.EnableExperimentalConcurrency);
1028+
addDeclKeywords(Sink, CurDeclContext, Context.LangOpts);
10241029
addStmtKeywords(Sink, CurDeclContext, MaybeFuncBody);
10251030
addClosureSignatureKeywordsIfApplicable(Sink, CurDeclContext);
10261031

@@ -1122,8 +1127,7 @@ void CodeCompletionCallbacksImpl::addKeywords(CodeCompletionResultSink &Sink,
11221127
.Default(false);
11231128
}) != ParsedKeywords.end();
11241129
if (!HasDeclIntroducer) {
1125-
addDeclKeywords(Sink, CurDeclContext,
1126-
Context.LangOpts.EnableExperimentalConcurrency);
1130+
addDeclKeywords(Sink, CurDeclContext, Context.LangOpts);
11271131
addLetVarKeywords(Sink);
11281132
}
11291133
break;

lib/IDE/CompletionLookup.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3023,7 +3023,7 @@ void CompletionLookup::getGenericRequirementCompletions(
30233023
}
30243024

30253025
bool CompletionLookup::canUseAttributeOnDecl(DeclAttrKind DAK, bool IsInSil,
3026-
bool IsConcurrencyEnabled,
3026+
const LangOptions &langOpts,
30273027
std::optional<DeclKind> DK,
30283028
StringRef Name) {
30293029
if (DeclAttribute::isUserInaccessible(DAK))
@@ -3034,8 +3034,12 @@ bool CompletionLookup::canUseAttributeOnDecl(DeclAttrKind DAK, bool IsInSil,
30343034
return false;
30353035
if (!IsInSil && DeclAttribute::isSilOnly(DAK))
30363036
return false;
3037-
if (!IsConcurrencyEnabled && DeclAttribute::isConcurrencyOnly(DAK))
3037+
if (!langOpts.EnableExperimentalConcurrency
3038+
&& DeclAttribute::isConcurrencyOnly(DAK))
30383039
return false;
3040+
if (auto feature = DeclAttribute::getRequiredFeature(DAK))
3041+
if (!langOpts.hasFeature(*feature))
3042+
return false;
30393043
if (!DK.has_value())
30403044
return true;
30413045
// Hide underscored attributes even if they are not marked as user
@@ -3059,11 +3063,10 @@ void CompletionLookup::getAttributeDeclCompletions(bool IsInSil,
30593063
#include "swift/AST/DeclNodes.def"
30603064
}
30613065
}
3062-
bool IsConcurrencyEnabled = Ctx.LangOpts.EnableExperimentalConcurrency;
30633066
std::string Description = TargetName.str() + " Attribute";
30643067
#define DECL_ATTR_ALIAS(KEYWORD, NAME) DECL_ATTR(KEYWORD, NAME, 0, 0)
30653068
#define DECL_ATTR(KEYWORD, NAME, ...) \
3066-
if (canUseAttributeOnDecl(DeclAttrKind::NAME, IsInSil, IsConcurrencyEnabled, \
3069+
if (canUseAttributeOnDecl(DeclAttrKind::NAME, IsInSil, Ctx.LangOpts, \
30673070
DK, #KEYWORD)) \
30683071
addDeclAttrKeyword(#KEYWORD, Description);
30693072
#include "swift/AST/DeclAttr.def"

lib/Sema/TypeCheckAttr.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1789,6 +1789,18 @@ void TypeChecker::checkDeclAttributes(Decl *D) {
17891789
for (auto attr : D->getExpandedAttrs()) {
17901790
if (!attr->isValid()) continue;
17911791

1792+
// If the attribute requires a feature that is not enabled, and it is not
1793+
// an implicit attribute, diagnose and disable it.
1794+
if (auto feature = DeclAttribute::getRequiredFeature(attr->getKind())) {
1795+
if (!attr->isImplicit()
1796+
&& !D->getASTContext().LangOpts.hasFeature(*feature)) {
1797+
Checker.diagnoseAndRemoveAttr(attr, diag::requires_experimental_feature,
1798+
attr->getAttrName(), false,
1799+
getFeatureName(*feature));
1800+
continue;
1801+
}
1802+
}
1803+
17921804
// If Attr.def says that the attribute cannot appear on this kind of
17931805
// declaration, diagnose it and disable it.
17941806
if (attr->canAppearOnDecl(D)) {
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// This contains code completion test cases for features covered by experimental
2+
// feature flags, and tests both the case when the feature is disabled and when
3+
// it's enabled. When a feature becomes non-experimental, move its test cases
4+
// into the normal complete_decl_attribute.swift test file.
5+
6+
// REQUIRES: asserts
7+
8+
// RUN: %batch-code-completion -filecheck-additional-suffix _DISABLED
9+
// RUN: %batch-code-completion -filecheck-additional-suffix _ENABLED -enable-experimental-feature ABIAttribute
10+
11+
// NOTE: Please do not include the ", N items" after "Begin completions". The
12+
// item count creates needless merge conflicts given that an "End completions"
13+
// line exists for each test.
14+
15+
@#^KEYWORD2^# func method(){}
16+
17+
// KEYWORD2: Begin completions
18+
// KEYWORD2_ENABLED-DAG: Keyword/None: abi[#Func Attribute#]; name=abi
19+
// KEYWORD2_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
20+
// KEYWORD2: End completions
21+
22+
@#^KEYWORD3^# class C {}
23+
24+
// KEYWORD3: Begin completions
25+
// KEYWORD3_ENABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
26+
// KEYWORD3_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
27+
// KEYWORD3: End completions
28+
29+
@#^KEYWORD3_2?check=KEYWORD3^#IB class C2 {}
30+
// Same as KEYWORD3.
31+
32+
@#^KEYWORD4^# enum E {}
33+
// KEYWORD4: Begin completions
34+
// KEYWORD4_ENABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
35+
// KEYWORD4_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
36+
// KEYWORD4: End completions
37+
38+
@#^KEYWORD5^# struct S{}
39+
// KEYWORD5: Begin completions
40+
// KEYWORD5_ENABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
41+
// KEYWORD5_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
42+
// KEYWORD5: End completions
43+
44+
@#^ON_GLOBALVAR^# var globalVar
45+
// ON_GLOBALVAR: Begin completions
46+
// ON_GLOBALVAR_ENABLED-DAG: Keyword/None: abi[#Var Attribute#]; name=abi
47+
// ON_GLOBALVAR_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
48+
// ON_GLOBALVAR: End completions
49+
50+
struct _S {
51+
@#^ON_INIT^# init()
52+
// ON_INIT: Begin completions
53+
// ON_INIT_ENABLED-DAG: Keyword/None: abi[#Constructor Attribute#]; name=abi
54+
// ON_INIT_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
55+
// ON_INIT: End completions
56+
57+
@#^ON_PROPERTY^# var foo
58+
// ON_PROPERTY: Begin completions
59+
// ON_PROPERTY_ENABLED-DAG: Keyword/None: abi[#Var Attribute#]; name=abi
60+
// ON_PROPERTY_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
61+
// ON_PROPERTY: End completions
62+
63+
@#^ON_METHOD^# private
64+
func foo()
65+
// ON_METHOD: Begin completions
66+
// ON_METHOD_ENABLED-DAG: Keyword/None: abi[#Func Attribute#]; name=abi
67+
// ON_METHOD_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
68+
// ON_METHOD: End completions
69+
70+
71+
func bar(@#^ON_PARAM_1?check=ON_PARAM^#)
72+
// ON_PARAM: Begin completions
73+
// ON_PARAM_ENABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
74+
// ON_PARAM_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
75+
// ON_PARAM: End completions
76+
77+
func bar(
78+
@#^ON_PARAM_2?check=ON_PARAM^#
79+
80+
arg: Int
81+
)
82+
// Same as ON_PARAM.
83+
84+
@#^ON_MEMBER_INDEPENDENT_1?check=ON_MEMBER_LAST^#
85+
86+
func dummy1() {}
87+
// Same as ON_MEMBER_LAST.
88+
89+
@#^ON_MEMBER_INDEPENDENT_2?check=ON_MEMBER_LAST^#
90+
func dummy2() {}
91+
// Same as ON_MEMBER_LAST.
92+
93+
94+
@#^ON_MEMBER_LAST^#
95+
// ON_MEMBER_LAST: Begin completions
96+
// ON_MEMBER_LAST_ENABLED-DAG: Keyword/None: abi[#Declaration Attribute#]; name=abi
97+
// ON_MEMBER_LAST_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
98+
// ON_MEMBER_LAST: End completions
99+
}
100+
101+
func takeClosure(_: () -> Void) {
102+
takeClosure { @#^IN_CLOSURE^# in
103+
print("x")
104+
}
105+
}
106+
// IN_CLOSURE: Begin completions
107+
// FIXME: Not valid in this position (but CompletionLookup can't tell that)
108+
// IN_CLOSURE_ENABLED-DAG: Keyword/None: abi[#Declaration Attribute#]; name=abi
109+
// IN_CLOSURE_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi
110+
// IN_CLOSURE: End completions
111+
112+
@#^KEYWORD_INDEPENDENT_1?check=KEYWORD_LAST^#
113+
114+
func dummy1() {}
115+
// Same as KEYWORD_LAST.
116+
117+
@#^KEYWORD_INDEPENDENT_2?check=KEYWORD_LAST^#
118+
func dummy2() {}
119+
// Same as KEYWORD_LAST.
120+
121+
@#^KEYWORD_LAST^#
122+
123+
// KEYWORD_LAST: Begin completions
124+
// KEYWORD_LAST_ENABLED-DAG: Keyword/None: abi[#Declaration Attribute#]; name=abi
125+
// KEYWORD_LAST_DISABLED-NOT: Keyword/None: abi[#Declaration Attribute#]; name=abi
126+
// KEYWORD_LAST: End completions

test/Misc/verify-swift-feature-testing.test-sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ EXCEPTIONAL_FILES = [
2020
pathlib.Path("test/ModuleInterface/swift-export-as.swift"),
2121
# Uses the pseudo-feature AvailabilityMacro=
2222
pathlib.Path("test/Sema/availability_define.swift"),
23+
# Tests behavior when you try to use a feature without enabling it
24+
pathlib.Path("test/attr/feature_requirement.swift"),
25+
# Tests completion with features both enabled and disabled
26+
pathlib.Path("test/IDE/complete_decl_attribute_feature_requirement.swift"),
2327
]
2428

2529
FEATURE_USAGE_RE = re.compile(

test/attr/feature_requirement.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// RUN: %target-typecheck-verify-swift -parse-as-library -verify-additional-prefix disabled-
2+
// RUN: %target-typecheck-verify-swift -parse-as-library -verify-additional-prefix enabled- -enable-experimental-feature ABIAttribute
3+
4+
// REQUIRES: asserts
5+
6+
// This test checks whether DECL_ATTR_FEATURE_REQUIREMENT is being applied correctly.
7+
// It is expected to need occasional edits as experimental features are stabilized.
8+
9+
@abi(func fn())
10+
func fn() {} // expected-disabled-error@-1 {{'abi' attribute is only valid when experimental feature ABIAttribute is enabled}}
11+
12+
#if hasAttribute(abi)
13+
#error("does have @abi") // expected-enabled-error {{does have @abi}}
14+
#else
15+
#error("doesn't have @abi") // expected-disabled-error {{doesn't have @abi}}
16+
#endif

tools/swift-ide-test/swift-ide-test.cpp

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,11 @@ FileCheckPath("filecheck", llvm::cl::value_desc("path"),
457457
static llvm::cl::opt<bool>
458458
SkipFileCheck("skip-filecheck", llvm::cl::desc("Skip 'FileCheck' checking"),
459459
llvm::cl::cat(Category));
460+
static llvm::cl::opt<std::string>
461+
FileCheckSuffix("filecheck-additional-suffix",
462+
llvm::cl::value_desc("check-prefix-suffix"),
463+
llvm::cl::desc("Additional suffix to add to check prefixes as an alternative"),
464+
llvm::cl::cat(Category));
460465

461466
// '-code-completion' options.
462467

@@ -1537,11 +1542,32 @@ static int doBatchCodeCompletion(const CompilerInvocation &InitInvok,
15371542
options::CodeCompletionToken != Token.Name)
15381543
continue;
15391544

1545+
SmallVector<std::string, 4> expandedCheckPrefixes;
1546+
15401547
llvm::errs() << "----\n";
15411548
llvm::errs() << "Token: " << Token.Name << "; offset=" << Token.Offset
15421549
<< "; pos=" << Token.Line << ":" << Token.Column;
1543-
for (auto Prefix : Token.CheckPrefixes) {
1544-
llvm::errs() << "; check=" << Prefix;
1550+
for (auto joinedPrefix : Token.CheckPrefixes) {
1551+
if (options::FileCheckSuffix.empty()) {
1552+
// Simple case: just copy what we have
1553+
expandedCheckPrefixes.push_back(joinedPrefix.str());
1554+
} else {
1555+
// For each comma-separated prefix, insert a variant with the suffix
1556+
// added to it: "X,Y" with suffix "_FOO" -> "X,X_FOO,Y,Y_FOO"
1557+
std::string expandedPrefix;
1558+
llvm::raw_string_ostream os(expandedPrefix);
1559+
1560+
SmallVector<StringRef, 4> splitPrefix;
1561+
joinedPrefix.split(splitPrefix, ',');
1562+
1563+
llvm::interleaveComma(splitPrefix, os, [&](StringRef prefix) {
1564+
os << prefix << ',' << prefix << options::FileCheckSuffix;
1565+
});
1566+
1567+
expandedCheckPrefixes.push_back(expandedPrefix);
1568+
}
1569+
1570+
llvm::errs() << "; check=" << expandedCheckPrefixes.back();
15451571
}
15461572
llvm::errs() << "\n";
15471573

@@ -1662,7 +1688,7 @@ static int doBatchCodeCompletion(const CompilerInvocation &InitInvok,
16621688
assert(!options::FileCheckPath.empty());
16631689

16641690
bool isFileCheckFailed = false;
1665-
for (auto Prefix : Token.CheckPrefixes) {
1691+
for (auto Prefix : expandedCheckPrefixes) {
16661692
StringRef FileCheckArgs[] = {options::FileCheckPath, SourceFilename,
16671693
"--check-prefixes", Prefix,
16681694
"--input-file", resultFilename};

0 commit comments

Comments
 (0)