Skip to content

Commit 69fb104

Browse files
committed
Diagnose access note @objc failures using remarks
This commit adds an ObjCReason::ExplicitlyObjCByAccessNote value which diagnoses invalid uses, but using remarks instead of errors so that the failures don’t block builds even with -warnings-as-errors enabled. This commit also adds annotations to attr/attr_objc.swift to generate a ton of access note test cases from it. In this commit, many of these test cases don’t pass yet. Subsequent commits will fix these bugs.
1 parent 164b3dc commit 69fb104

File tree

8 files changed

+712
-458
lines changed

8 files changed

+712
-458
lines changed

include/swift/AST/Attr.h

+17-2
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ class DeclAttribute : public AttributeBase {
118118
Invalid : 1
119119
);
120120

121-
SWIFT_INLINE_BITFIELD(ObjCAttr, DeclAttribute, 1+1+1,
121+
SWIFT_INLINE_BITFIELD(ObjCAttr, DeclAttribute, 1+1+1+1,
122122
/// Whether this attribute has location information that trails the main
123123
/// record, which contains the locations of the parentheses and any names.
124124
HasTrailingLocationInfo : 1,
@@ -128,7 +128,10 @@ class DeclAttribute : public AttributeBase {
128128

129129
/// Whether the @objc was inferred using Swift 3's deprecated inference
130130
/// rules.
131-
Swift3Inferred : 1
131+
Swift3Inferred : 1,
132+
133+
/// Whether the @objc was created by an access note.
134+
AddedByAccessNote : 1
132135
);
133136

134137
SWIFT_INLINE_BITFIELD(DynamicReplacementAttr, DeclAttribute, 1,
@@ -749,6 +752,7 @@ class ObjCAttr final : public DeclAttribute,
749752
Bits.ObjCAttr.HasTrailingLocationInfo = false;
750753
Bits.ObjCAttr.ImplicitName = implicitName;
751754
Bits.ObjCAttr.Swift3Inferred = false;
755+
Bits.ObjCAttr.AddedByAccessNote = false;
752756

753757
if (name) {
754758
NameData = name->getOpaqueValue();
@@ -868,6 +872,17 @@ class ObjCAttr final : public DeclAttribute,
868872
Bits.ObjCAttr.Swift3Inferred = inferred;
869873
}
870874

875+
/// Determine whether this attribute was added by an access note. If it is,
876+
/// the compiler will not treat it as a hard error if the attribute is
877+
/// invalid.
878+
bool getAddedByAccessNote() const {
879+
return Bits.ObjCAttr.AddedByAccessNote;
880+
}
881+
882+
void setAddedByAccessNote(bool accessNote = true) {
883+
Bits.ObjCAttr.AddedByAccessNote = accessNote;
884+
}
885+
871886
/// Clear the name of this entity.
872887
void clearName() {
873888
NameData = nullptr;

include/swift/AST/DiagnosticsSema.def

+1-1
Original file line numberDiff line numberDiff line change
@@ -4820,7 +4820,7 @@ ERROR(objc_extension_not_class,none,
48204820
"'@objc' can only be applied to an extension of a class", ())
48214821

48224822
// If you change this, also change enum ObjCReason
4823-
#define OBJC_ATTR_SELECT "select{marked @_cdecl|marked dynamic|marked @objc|marked @IBOutlet|marked @IBAction|marked @IBSegueAction|marked @NSManaged|a member of an @objc protocol|implicitly @objc|an @objc override|an implementation of an @objc requirement|marked @IBInspectable|marked @GKInspectable|in an @objc extension of a class (without @nonobjc)}"
4823+
#define OBJC_ATTR_SELECT "select{marked @_cdecl|marked dynamic|marked @objc|marked @IBOutlet|marked @IBAction|marked @IBSegueAction|marked @NSManaged|a member of an @objc protocol|implicitly @objc|an @objc override|an implementation of an @objc requirement|marked @IBInspectable|marked @GKInspectable|in an @objc extension of a class (without @nonobjc)|marked @objc by an access note}"
48244824

48254825
WARNING(attribute_meaningless_when_nonobjc,none,
48264826
"'@%0' attribute is meaningless on a property that cannot be "

lib/AST/Attr.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -1332,6 +1332,7 @@ SourceLoc ObjCAttr::getRParenLoc() const {
13321332
ObjCAttr *ObjCAttr::clone(ASTContext &context) const {
13331333
auto attr = new (context) ObjCAttr(getName(), isNameImplicit());
13341334
attr->setSwift3Inferred(isSwift3Inferred());
1335+
attr->setAddedByAccessNote(getAddedByAccessNote());
13351336
return attr;
13361337
}
13371338

lib/Sema/TypeCheckDeclObjC.cpp

+23-8
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ swift::behaviorLimitForObjCReason(ObjCReason reason, ASTContext &ctx) {
5454
return DiagnosticBehavior::Unspecified;
5555
return DiagnosticBehavior::Ignore;
5656

57+
case ObjCReason::ExplicitlyObjCByAccessNote:
58+
return DiagnosticBehavior::Remark;
59+
5760
case ObjCReason::MemberOfObjCSubclass:
5861
case ObjCReason::MemberOfObjCMembersClass:
5962
case ObjCReason::ElementOfObjCEnum:
@@ -79,6 +82,7 @@ unsigned swift::getObjCDiagnosticAttrKind(ObjCReason reason) {
7982
case ObjCReason::ExplicitlyIBInspectable:
8083
case ObjCReason::ExplicitlyGKInspectable:
8184
case ObjCReason::MemberOfObjCExtension:
85+
case ObjCReason::ExplicitlyObjCByAccessNote:
8286
return static_cast<unsigned>(reason);
8387

8488
case ObjCReason::MemberOfObjCSubclass:
@@ -114,6 +118,10 @@ static void describeObjCReason(const ValueDecl *VD, ObjCReason Reason) {
114118
cast<ProtocolDecl>(requirement->getDeclContext())
115119
->getName());
116120
}
121+
else if (Reason == ObjCReason::ExplicitlyObjCByAccessNote) {
122+
// FIXME: Look up the Reason string in the access note and emit a note that
123+
// includes it
124+
}
117125
}
118126

119127
static void diagnoseTypeNotRepresentableInObjC(const DeclContext *DC,
@@ -1119,6 +1127,13 @@ static bool isMemberOfObjCMembersClass(const ValueDecl *VD) {
11191127
return classDecl->checkAncestry(AncestryFlags::ObjCMembers);
11201128
}
11211129

1130+
static ObjCReason reasonForObjCAttr(const ObjCAttr *attr) {
1131+
if (attr->getAddedByAccessNote())
1132+
return ObjCReason::ExplicitlyObjCByAccessNote;
1133+
1134+
return ObjCReason::ExplicitlyObjC;
1135+
}
1136+
11221137
// A class is @objc if it does not have generic ancestry, and it either has
11231138
// an explicit @objc attribute, or its superclass is @objc.
11241139
static Optional<ObjCReason> shouldMarkClassAsObjC(const ClassDecl *CD) {
@@ -1191,7 +1206,7 @@ static Optional<ObjCReason> shouldMarkClassAsObjC(const ClassDecl *CD) {
11911206
}
11921207
}
11931208

1194-
return ObjCReason(ObjCReason::ExplicitlyObjC);
1209+
return reasonForObjCAttr(CD->getAttrs().getAttribute<ObjCAttr>());
11951210
}
11961211

11971212
if (ancestry.contains(AncestryFlags::ObjC)) {
@@ -1272,8 +1287,8 @@ Optional<ObjCReason> shouldMarkAsObjC(const ValueDecl *VD, bool allowImplicit) {
12721287
};
12731288

12741289
// explicitly declared @objc.
1275-
if (VD->getAttrs().hasAttribute<ObjCAttr>())
1276-
return ObjCReason(ObjCReason::ExplicitlyObjC);
1290+
if (auto attr = VD->getAttrs().getAttribute<ObjCAttr>())
1291+
return reasonForObjCAttr(attr);
12771292
// Getter or setter for an @objc property or subscript.
12781293
if (auto accessor = dyn_cast<AccessorDecl>(VD)) {
12791294
if (accessor->getAccessorKind() == AccessorKind::Get ||
@@ -1486,18 +1501,18 @@ bool IsObjCRequest::evaluate(Evaluator &evaluator, ValueDecl *VD) const {
14861501
// Enums can be @objc so long as they have a raw type that is representable
14871502
// as an arithmetic type in C.
14881503
if (isEnumObjC(enumDecl))
1489-
isObjC = ObjCReason(ObjCReason::ExplicitlyObjC);
1504+
isObjC = reasonForObjCAttr(enumDecl->getAttrs().getAttribute<ObjCAttr>());
14901505
} else if (auto enumElement = dyn_cast<EnumElementDecl>(VD)) {
14911506
// Enum elements can be @objc so long as the containing enum is @objc.
14921507
if (enumElement->getParentEnum()->isObjC()) {
1493-
if (enumElement->getAttrs().hasAttribute<ObjCAttr>())
1494-
isObjC = ObjCReason::ExplicitlyObjC;
1508+
if (auto attr = enumElement->getAttrs().getAttribute<ObjCAttr>())
1509+
isObjC = reasonForObjCAttr(attr);
14951510
else
14961511
isObjC = ObjCReason::ElementOfObjCEnum;
14971512
}
14981513
} else if (auto proto = dyn_cast<ProtocolDecl>(VD)) {
1499-
if (proto->getAttrs().hasAttribute<ObjCAttr>()) {
1500-
isObjC = ObjCReason(ObjCReason::ExplicitlyObjC);
1514+
if (auto attr = proto->getAttrs().getAttribute<ObjCAttr>()) {
1515+
isObjC = reasonForObjCAttr(attr);
15011516

15021517
// If the protocol is @objc, it may only refine other @objc protocols.
15031518
// FIXME: Revisit this restriction.

lib/Sema/TypeCheckDeclPrimary.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -1470,7 +1470,9 @@ static void applyAccessNote(ValueDecl *VD, const AccessNote &note,
14701470
ASTContext &ctx = VD->getASTContext();
14711471

14721472
addOrRemoveAttr<ObjCAttr>(VD, notes, note.ObjC, [&]{
1473-
return ObjCAttr::create(ctx, note.ObjCName, false);
1473+
auto attr = ObjCAttr::create(ctx, note.ObjCName, false);
1474+
attr->setAddedByAccessNote();
1475+
return attr;
14741476
});
14751477

14761478
addOrRemoveAttr<DynamicAttr>(VD, notes, note.Dynamic, [&]{

lib/Sema/TypeCheckObjC.h

+3
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ class ObjCReason {
6969
ExplicitlyGKInspectable,
7070
/// Is it a member of an @objc extension of a class.
7171
MemberOfObjCExtension,
72+
/// Has an explicit '@objc' attribute added by an access note, rather than
73+
/// written in source code.
74+
ExplicitlyObjCByAccessNote,
7275

7376
// These kinds do not appear in diagnostics.
7477

test/attr/Inputs/access-note-gen.py

+153
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
#!/usr/bin/env python
2+
# coding=utf8
3+
4+
"""
5+
Convert selected @objc attributes in a source file into access notes, removing
6+
the originals in the process.
7+
"""
8+
9+
from __future__ import print_function
10+
11+
import io
12+
import re
13+
import sys
14+
15+
#
16+
# Entry point
17+
#
18+
19+
20+
def main():
21+
if len(sys.argv) != 4:
22+
print('Too few args to ' + sys.argv[0])
23+
print('Usage: access-note-gen.py <input-file> <output-source-file> ' +
24+
'<output-access-notes-file>')
25+
sys.exit(1)
26+
27+
with io.open(sys.argv[1], mode='r', encoding='utf8') as input_file, \
28+
io.open(sys.argv[2], mode='w', encoding='utf8') as output_file, \
29+
io.open(sys.argv[3], mode='w', encoding='utf8') as access_notes_file:
30+
31+
# Add header to access notes file
32+
access_notes_file.write(u"""\
33+
Reason: 'fancy tests'
34+
Notes:""")
35+
36+
# Loop over input lines, transforming them into output lines, writing access
37+
# notes as a side effect.
38+
for input_line in input_file:
39+
# Look for access-note-move comments.
40+
input_line = access_note_move_re.sub(replacer(move_at_objc_to_access_note,
41+
access_notes_file),
42+
input_line, count=1)
43+
44+
# Look for access-note-adjust comments.
45+
input_line = adjust_comment_re.sub(replacer(adjust_comments),
46+
input_line, count=1)
47+
48+
output_file.write(input_line)
49+
50+
51+
#
52+
# Offsets
53+
#
54+
55+
"""Matches an @±N offset."""
56+
offset_re_fragment = r'[ \t]*(?:@([+-]\d+))?[ \t]*'
57+
58+
59+
def offsetify(*offsets):
60+
"""Sum line offsets matched by offset_re_fragment and convert them to strings
61+
like @+3 or @-2."""
62+
63+
offset = sum([int(o) for o in offsets if o is not None])
64+
if offset < 0:
65+
return u"@-" + str(-offset)
66+
elif offset > 0:
67+
return u"@+" + str(offset)
68+
else:
69+
return u""
70+
71+
72+
#
73+
# Adjusting comments
74+
#
75+
76+
"""Matches expected-warning/note/remark and its offset."""
77+
expected_other_diag_re = re.compile(r'expected-(warning|note|remark)' +
78+
offset_re_fragment)
79+
80+
"""Matches expected-error and its offset."""
81+
expected_error_re = re.compile(r'expected-error' + offset_re_fragment)
82+
83+
"""Matches the string 'marked @objc'."""
84+
marked_objc_re = re.compile(r'marked @objc')
85+
86+
"""Matches any non-none fix-it expectation."""
87+
fixit_re = re.compile(r'{{\d+-\d+=[^}]*}}')
88+
89+
90+
def adjust_comments(offset, comment_str):
91+
"""Replace expected-errors with expected-remarks, and make other adjustments
92+
to diagnostics so that they reflect access notes."""
93+
94+
adjusted = expected_other_diag_re.sub(lambda m: u"expected-" + m.group(1) +
95+
offsetify(offset, m.group(2)),
96+
comment_str)
97+
adjusted = expected_error_re.sub(lambda m: u"expected-remark" +
98+
offsetify(offset, m.group(1)),
99+
adjusted)
100+
adjusted = marked_objc_re.sub(u"marked @objc by an access note", adjusted)
101+
adjusted = fixit_re.sub(u"{{none}}", adjusted)
102+
103+
return u"// [expectations adjusted] " + adjusted
104+
105+
106+
#
107+
# Writing attrs to access notes
108+
#
109+
110+
def move_at_objc_to_access_note(access_notes_file, arg, offset, access_note_name):
111+
"""Write an @objc attribute into an access notes file, then return the
112+
string that will replace the attribute and trailing comment."""
113+
114+
access_notes_file.write(u"""
115+
- Name: '{}'
116+
ObjC: true""".format(access_note_name))
117+
if arg:
118+
access_notes_file.write(u"""
119+
ObjCName: '{}'""".format(arg))
120+
121+
# Default to shifting expected diagnostics down 1 line.
122+
if offset is None:
123+
offset = 1
124+
125+
return u"// access-note-adjust" + offsetify(offset) + u" [attr moved] " + \
126+
u"expected-remark{{access note for fancy tests adds attribute 'objc' to " + \
127+
u"this }} expected-note{{add attribute explicitly to silence this warning}}"
128+
129+
130+
#
131+
# Matching lines
132+
#
133+
134+
"""Matches '@objc(foo) // access-note-move{{access-note-name}}'
135+
or '@objc // access-note-move{{access-note-name}}'"""
136+
access_note_move_re = re.compile(r'@objc(?:\(([\w:]+)\))?[ \t]*' +
137+
r'//[ \t]*access-note-move' +
138+
offset_re_fragment +
139+
r'\{\{([^}]*)\}\}')
140+
141+
"""Matches // access-note-adjust <comment>"""
142+
adjust_comment_re = re.compile(r'//[ \t]*access-note-adjust' + offset_re_fragment +
143+
r'(.*)')
144+
145+
146+
def replacer(fn, *args):
147+
"""Returns a lambda which calls fn with args, followed by the groups from
148+
the match passed to the lambda."""
149+
150+
return lambda m: fn(*(args + m.groups()))
151+
152+
153+
main()

0 commit comments

Comments
 (0)