r241547 - Warn when an intended Objective-C specialization was actually a useless protocol qualification.
Douglas Gregor
dgregor at apple.com
Mon Jul 6 20:58:28 PDT 2015
Author: dgregor
Date: Mon Jul 6 22:58:28 2015
New Revision: 241547
URL: http://llvm.org/viewvc/llvm-project?rev=241547&view=rev
Log:
Warn when an intended Objective-C specialization was actually a useless protocol qualification.
Warn in cases where one has provided redundant protocol qualification
that might be a typo for a specialization, e.g., NSArray<NSObject>,
which is pointless (NSArray declares that it conforms to NSObject) and
is likely to be a typo for NSArray<NSObject *>, i.e., an array of
NSObject pointers. This warning is very narrow, only applying when the
base type being qualified is parameterized, has the same number of
parameters as their are protocols listed, all of the names can also
refer to types (including Objective-C class types, of course), and at
least one of those types is an Objective-C class (making this a typo
for a missing '*'). The limitations are partly for performance reasons
(we don't want to do redundant name lookup unless we really need to),
and because we want the warning to apply in very limited cases to
limit false positives.
Part of rdar://problem/6294649.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseObjc.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/test/SemaObjC/parameterized_classes_subst.m
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=241547&r1=241546&r2=241547&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Jul 6 22:58:28 2015
@@ -280,6 +280,7 @@ def FunctionDefInObjCContainer : DiagGro
def BadFunctionCast : DiagGroup<"bad-function-cast">;
def ObjCPropertyImpl : DiagGroup<"objc-property-implementation">;
def ObjCPropertyNoAttribute : DiagGroup<"objc-property-no-attribute">;
+def ObjCProtocolQualifiers : DiagGroup<"objc-protocol-qualifiers">;
def ObjCMissingSuperCalls : DiagGroup<"objc-missing-super-calls">;
def ObjCDesignatedInit : DiagGroup<"objc-designated-initializers">;
def ObjCRetainBlockProperty : DiagGroup<"objc-noncopy-retain-block-property">;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=241547&r1=241546&r2=241547&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul 6 22:58:28 2015
@@ -7824,4 +7824,8 @@ def err_objc_type_arg_not_id_compatible
def err_objc_type_arg_does_not_match_bound : Error<
"type argument %0 does not satisfy the bound (%1) of type parameter %2">;
+def warn_objc_redundant_qualified_class_type : Warning<
+ "parameterized class %0 already conforms to the protocols listed; did you "
+ "forget a '*'?">, InGroup<ObjCProtocolQualifiers>;
+
} // end of sema component.
Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=241547&r1=241546&r2=241547&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Mon Jul 6 22:58:28 2015
@@ -1274,6 +1274,7 @@ private:
/// Objective-C object or object pointer type, which may be either
/// type arguments or protocol qualifiers.
void parseObjCTypeArgsOrProtocolQualifiers(
+ ParsedType baseType,
SourceLocation &typeArgsLAngleLoc,
SmallVectorImpl<ParsedType> &typeArgs,
SourceLocation &typeArgsRAngleLoc,
@@ -1287,6 +1288,7 @@ private:
/// Parse either Objective-C type arguments or protocol qualifiers; if the
/// former, also parse protocol qualifiers afterward.
void parseObjCTypeArgsAndProtocolQualifiers(
+ ParsedType baseType,
SourceLocation &typeArgsLAngleLoc,
SmallVectorImpl<ParsedType> &typeArgs,
SourceLocation &typeArgsRAngleLoc,
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=241547&r1=241546&r2=241547&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Jul 6 22:58:28 2015
@@ -7194,6 +7194,7 @@ public:
/// arguments, as appropriate.
void actOnObjCTypeArgsOrProtocolQualifiers(
Scope *S,
+ ParsedType baseType,
SourceLocation lAngleLoc,
ArrayRef<IdentifierInfo *> identifiers,
ArrayRef<SourceLocation> identifierLocs,
Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=241547&r1=241546&r2=241547&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseObjc.cpp (original)
+++ cfe/trunk/lib/Parse/ParseObjc.cpp Mon Jul 6 22:58:28 2015
@@ -319,7 +319,8 @@ Decl *Parser::ParseObjCAtInterfaceDeclar
// Type arguments for the superclass or protocol conformances.
if (Tok.is(tok::less)) {
- parseObjCTypeArgsOrProtocolQualifiers(typeArgsLAngleLoc,
+ parseObjCTypeArgsOrProtocolQualifiers(ParsedType(),
+ typeArgsLAngleLoc,
typeArgs,
typeArgsRAngleLoc,
LAngleLoc,
@@ -1589,6 +1590,7 @@ TypeResult Parser::parseObjCProtocolQual
/// '<' type-name '...'[opt] (',' type-name '...'[opt])* '>'
///
void Parser::parseObjCTypeArgsOrProtocolQualifiers(
+ ParsedType baseType,
SourceLocation &typeArgsLAngleLoc,
SmallVectorImpl<ParsedType> &typeArgs,
SourceLocation &typeArgsRAngleLoc,
@@ -1648,6 +1650,7 @@ void Parser::parseObjCTypeArgsOrProtocol
// Let Sema figure out what we parsed.
Actions.actOnObjCTypeArgsOrProtocolQualifiers(getCurScope(),
+ baseType,
lAngleLoc,
identifiers,
identifierLocs,
@@ -1723,6 +1726,7 @@ void Parser::parseObjCTypeArgsOrProtocol
}
void Parser::parseObjCTypeArgsAndProtocolQualifiers(
+ ParsedType baseType,
SourceLocation &typeArgsLAngleLoc,
SmallVectorImpl<ParsedType> &typeArgs,
SourceLocation &typeArgsRAngleLoc,
@@ -1734,7 +1738,8 @@ void Parser::parseObjCTypeArgsAndProtoco
assert(Tok.is(tok::less));
// Parse the first angle-bracket-delimited clause.
- parseObjCTypeArgsOrProtocolQualifiers(typeArgsLAngleLoc,
+ parseObjCTypeArgsOrProtocolQualifiers(baseType,
+ typeArgsLAngleLoc,
typeArgs,
typeArgsRAngleLoc,
protocolLAngleLoc,
@@ -1786,7 +1791,7 @@ TypeResult Parser::parseObjCTypeArgsAndP
SourceLocation protocolRAngleLoc;
// Parse type arguments and protocol qualifiers.
- parseObjCTypeArgsAndProtocolQualifiers(typeArgsLAngleLoc, typeArgs,
+ parseObjCTypeArgsAndProtocolQualifiers(type, typeArgsLAngleLoc, typeArgs,
typeArgsRAngleLoc, protocolLAngleLoc,
protocols, protocolLocs,
protocolRAngleLoc, consumeLastToken);
Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=241547&r1=241546&r2=241547&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Mon Jul 6 22:58:28 2015
@@ -1220,6 +1220,7 @@ class ObjCTypeArgOrProtocolValidatorCCC
void Sema::actOnObjCTypeArgsOrProtocolQualifiers(
Scope *S,
+ ParsedType baseType,
SourceLocation lAngleLoc,
ArrayRef<IdentifierInfo *> identifiers,
ArrayRef<SourceLocation> identifierLocs,
@@ -1237,6 +1238,27 @@ void Sema::actOnObjCTypeArgsOrProtocolQu
auto resolvedAsProtocols = [&] {
assert(numProtocolsResolved == identifiers.size() && "Unresolved protocols");
+ // Determine whether the base type is a parameterized class, in
+ // which case we want to warn about typos such as
+ // "NSArray<NSObject>" (that should be NSArray<NSObject *>).
+ ObjCInterfaceDecl *baseClass = nullptr;
+ QualType base = GetTypeFromParser(baseType, nullptr);
+ bool allAreTypeNames = false;
+ SourceLocation firstClassNameLoc;
+ if (!base.isNull()) {
+ if (const auto *objcObjectType = base->getAs<ObjCObjectType>()) {
+ baseClass = objcObjectType->getInterface();
+ if (baseClass) {
+ if (auto typeParams = baseClass->getTypeParamList()) {
+ if (typeParams->size() == numProtocolsResolved) {
+ // Note that we should be looking for type names, too.
+ allAreTypeNames = true;
+ }
+ }
+ }
+ }
+ }
+
for (unsigned i = 0, n = protocols.size(); i != n; ++i) {
ObjCProtocolDecl *&proto
= reinterpret_cast<ObjCProtocolDecl *&>(protocols[i]);
@@ -1261,6 +1283,47 @@ void Sema::actOnObjCTypeArgsOrProtocolQu
Diag(forwardDecl->getLocation(), diag::note_protocol_decl_undefined)
<< forwardDecl;
}
+
+ // If everything this far has been a type name (and we care
+ // about such things), check whether this name refers to a type
+ // as well.
+ if (allAreTypeNames) {
+ if (auto *decl = LookupSingleName(S, identifiers[i], identifierLocs[i],
+ LookupOrdinaryName)) {
+ if (isa<ObjCInterfaceDecl>(decl)) {
+ if (firstClassNameLoc.isInvalid())
+ firstClassNameLoc = identifierLocs[i];
+ } else if (!isa<TypeDecl>(decl)) {
+ // Not a type.
+ allAreTypeNames = false;
+ }
+ } else {
+ allAreTypeNames = false;
+ }
+ }
+ }
+
+ // All of the protocols listed also have type names, and at least
+ // one is an Objective-C class name. Check whether all of the
+ // protocol conformances are declared by the base class itself, in
+ // which case we warn.
+ if (allAreTypeNames && firstClassNameLoc.isValid()) {
+ llvm::SmallPtrSet<ObjCProtocolDecl*, 8> knownProtocols;
+ Context.CollectInheritedProtocols(baseClass, knownProtocols);
+ bool allProtocolsDeclared = true;
+ for (auto proto : protocols) {
+ if (knownProtocols.count(static_cast<ObjCProtocolDecl *>(proto)) == 0) {
+ allProtocolsDeclared = false;
+ break;
+ }
+ }
+
+ if (allProtocolsDeclared) {
+ Diag(firstClassNameLoc, diag::warn_objc_redundant_qualified_class_type)
+ << baseClass->getDeclName() << SourceRange(lAngleLoc, rAngleLoc)
+ << FixItHint::CreateInsertion(
+ PP.getLocForEndOfToken(firstClassNameLoc), " *");
+ }
}
protocolLAngleLoc = lAngleLoc;
Modified: cfe/trunk/test/SemaObjC/parameterized_classes_subst.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/parameterized_classes_subst.m?rev=241547&r1=241546&r2=241547&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/parameterized_classes_subst.m (original)
+++ cfe/trunk/test/SemaObjC/parameterized_classes_subst.m Mon Jul 6 22:58:28 2015
@@ -3,8 +3,11 @@
// Test the substitution of type arguments for type parameters when
// using parameterized classes in Objective-C.
+ at protocol NSObject
+ at end
+
__attribute__((objc_root_class))
- at interface NSObject
+ at interface NSObject <NSObject>
+ (instancetype)alloc;
- (instancetype)init;
@end
@@ -376,3 +379,8 @@ void test_ternary_operator(NSArray<NSStr
ip = [super array]; // expected-warning{{from 'NSArray<NSString *> *'}}
}
@end
+
+// --------------------------------------------------------------------------
+// warning about likely protocol/class name typos.
+// --------------------------------------------------------------------------
+typedef NSArray<NSObject> ArrayOfNSObjectWarning; // expected-warning{{parameterized class 'NSArray' already conforms to the protocols listed; did you forget a '*'?}}
More information about the cfe-commits
mailing list