[PATCH] D49462: [ObjC] Error out when using forward-declared protocol in a @protocol expression

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 161142.
arphaman marked 3 inline comments as done.
arphaman added a comment.

address review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D49462

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/CGObjCMac.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/forward-declare-protocol-gnu.m
  test/CodeGenObjC/forward-protocol-metadata-symbols.m
  test/CodeGenObjC/hidden-visibility.m
  test/CodeGenObjC/link-errors.m
  test/CodeGenObjC/protocol-comdat.m
  test/CodeGenObjC/protocols-lazy.m
  test/CodeGenObjC/protocols.m
  test/PCH/objc_exprs.h
  test/Parser/objc-cxx-keyword-identifiers.mm
  test/SemaObjC/protocol-expr-neg-1.m


Index: test/SemaObjC/protocol-expr-neg-1.m
===================================================================
--- test/SemaObjC/protocol-expr-neg-1.m
+++ test/SemaObjC/protocol-expr-neg-1.m
@@ -12,7 +12,7 @@
 int main()
 {
 	Protocol *proto = @protocol(p1);
-        Protocol *fproto = @protocol(fproto); // expected-warning {{@protocol is using a forward protocol declaration of 'fproto'}}
+        Protocol *fproto = @protocol(fproto); // expected-error {{@protocol is using a forward protocol declaration of 'fproto'}}
 	Protocol *pp = @protocol(i); // expected-error {{cannot find protocol declaration for 'i'}}
 	Protocol *p1p = @protocol(cl); // expected-error {{cannot find protocol declaration for 'cl'}}
 }
@@ -26,9 +26,9 @@
 @end
 
 int doesConform(id foo) {
-  return [foo conformsToProtocol:@protocol(TestProtocol)]; // expected-warning {{@protocol is using a forward protocol declaration of 'TestProtocol'}}
+  return [foo conformsToProtocol:@protocol(TestProtocol)]; // expected-error {{@protocol is using a forward protocol declaration of 'TestProtocol'}}
 }
 
 int doesConformSuper(id foo) {
-  return [foo conformsToProtocol:@protocol(SuperProtocol)]; // expected-warning {{@protocol is using a forward protocol declaration of 'SuperProtocol'}}
+  return [foo conformsToProtocol:@protocol(SuperProtocol)]; // expected-error {{@protocol is using a forward protocol declaration of 'SuperProtocol'}}
 }
Index: test/Parser/objc-cxx-keyword-identifiers.mm
===================================================================
--- test/Parser/objc-cxx-keyword-identifiers.mm
+++ test/Parser/objc-cxx-keyword-identifiers.mm
@@ -18,7 +18,9 @@
 @protocol new // expected-error {{expected identifier; 'new' is a keyword in Objective-C++}}
 @end
 
-@protocol P2, delete; // expected-error {{expected identifier; 'delete' is a keyword in Objective-C++}}
+@protocol P2;
+@protocol delete // expected-error {{expected identifier; 'delete' is a keyword in Objective-C++}}
+@end
 
 @class Foo, try; // expected-error {{expected identifier; 'try' is a keyword in Objective-C++}}
 
Index: test/PCH/objc_exprs.h
===================================================================
--- test/PCH/objc_exprs.h
+++ test/PCH/objc_exprs.h
@@ -1,11 +1,13 @@
 
 @protocol foo;
+@protocol foo2
+@end
 @class itf;
 
 // Expressions
 typedef typeof(@"foo" "bar") objc_string;
 typedef typeof(@encode(int)) objc_encode;
-typedef typeof(@protocol(foo)) objc_protocol;
+typedef typeof(@protocol(foo2)) objc_protocol;
 typedef typeof(@selector(noArgs)) objc_selector_noArgs;
 typedef typeof(@selector(oneArg:)) objc_selector_oneArg;
 typedef typeof(@selector(foo:bar:)) objc_selector_twoArg;
Index: test/CodeGenObjC/protocols.m
===================================================================
--- test/CodeGenObjC/protocols.m
+++ test/CodeGenObjC/protocols.m
@@ -7,7 +7,8 @@
 -(int) conformsTo: (id) x;
 @end
 
-@protocol P0;
+@protocol P0
+@end
 
 @protocol P1
 +(void) classMethodReq0;
Index: test/CodeGenObjC/protocols-lazy.m
===================================================================
--- test/CodeGenObjC/protocols-lazy.m
+++ test/CodeGenObjC/protocols-lazy.m
@@ -18,7 +18,10 @@
 // RUN: grep OBJC_PROTOCOL_P3 %t | count 3
 // RUN: not grep OBJC_PROTOCOL_INSTANCE_METHODS_P3 %t
 @protocol P3;
-void f1() { id x = @protocol(P3); }
+@interface UserP3<P3>
+@end
+@implementation UserP3
+@end
 
 // Definition triggered by class reference.
 // RUN: grep OBJC_PROTOCOL_P4 %t | count 3
@@ -31,10 +34,16 @@
 // RUN: grep OBJC_PROTOCOL_P5 %t | count 3
 // RUN: grep OBJC_PROTOCOL_INSTANCE_METHODS_P5 %t | count 3
 @protocol P5;
-void f2() { id x = @protocol(P5); } // This generates a forward
-                                    // reference, which has to be
-                                    // updated on the next line.
-@protocol P5 -im1; @end               
+@interface UserP5<P5> // This generates a forward
+                      // reference, which has to be
+                      // updated on the next line.
+@end
+@protocol P5 -im1; @end
+@implementation UserP5
+
+- im1 { }
+
+@end
 
 // Protocol reference following definition.
 // RUN: grep OBJC_PROTOCOL_P6 %t | count 4
Index: test/CodeGenObjC/protocol-comdat.m
===================================================================
--- test/CodeGenObjC/protocol-comdat.m
+++ test/CodeGenObjC/protocol-comdat.m
@@ -4,8 +4,8 @@
 - (void) method;
 @end
 
-@protocol Q;
-@protocol R;
+@protocol Q @end
+@protocol R @end
 
 @interface I<P>
 @end
Index: test/CodeGenObjC/link-errors.m
===================================================================
--- test/CodeGenObjC/link-errors.m
+++ test/CodeGenObjC/link-errors.m
@@ -10,7 +10,7 @@
 -(id) init;
 @end
 
-@protocol P;
+@protocol P @end
 
 @interface A : Root
 @end
Index: test/CodeGenObjC/hidden-visibility.m
===================================================================
--- test/CodeGenObjC/hidden-visibility.m
+++ test/CodeGenObjC/hidden-visibility.m
@@ -16,7 +16,7 @@
 @end
 
 
-@protocol Prot0;
+@protocol Prot0 @end
 
 id f0() {
   return @protocol(Prot0);
Index: test/CodeGenObjC/forward-protocol-metadata-symbols.m
===================================================================
--- test/CodeGenObjC/forward-protocol-metadata-symbols.m
+++ test/CodeGenObjC/forward-protocol-metadata-symbols.m
@@ -3,7 +3,7 @@
 
 @interface NSObject @end
 
-@protocol P0;
+@protocol P0 @end
 
 @interface A : NSObject <P0>
 +(Class) getClass;
@@ -19,8 +19,8 @@
 }
 
 // CHECK: @"\01l_OBJC_PROTOCOL_$_P0" = weak hidden global
-// CHECK: @"\01l_OBJC_CLASS_PROTOCOLS_$_A" = private global
 // CHECK: @"\01l_OBJC_LABEL_PROTOCOL_$_P0" = weak hidden global
+// CHECK: @"\01l_OBJC_CLASS_PROTOCOLS_$_A" = private global
 // CHECK: @"\01l_OBJC_PROTOCOL_REFERENCE_$_P0" = weak hidden global
 
 // CHECK: llvm.used = appending global [3 x i8*]
@@ -33,7 +33,7 @@
 // CHECK-SAME: OBJC_METH_VAR_NAME_
 // CHECK-SAME: OBJC_METH_VAR_TYPE_
 // CHECK-SAME: "\01l_OBJC_$_CLASS_METHODS_A"
-// CHECK-SAME: "\01l_OBJC_CLASS_PROTOCOLS_$_A"
 // CHECK-SAME: OBJC_CLASS_NAME_.1
+// CHECK-SAME: "\01l_OBJC_CLASS_PROTOCOLS_$_A"
 // CHECK-SAME: "OBJC_LABEL_CLASS_$"
 // CHECK-SAME: section "llvm.metadata"
Index: test/CodeGenObjC/forward-declare-protocol-gnu.m
===================================================================
--- test/CodeGenObjC/forward-declare-protocol-gnu.m
+++ test/CodeGenObjC/forward-declare-protocol-gnu.m
@@ -3,9 +3,11 @@
 // Regression test: check that we don't crash when referencing a forward-declared protocol.
 @protocol P;
 
-Protocol *getProtocol(void)
-{
-	        return @protocol(P);
-}
+@interface I <P>
+@end
+
+@implementation I
+
+@end
 
 // CHECK: @.objc_protocol
Index: lib/Sema/SemaExprObjC.cpp
===================================================================
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1227,8 +1227,12 @@
     Diag(ProtoLoc, diag::err_undeclared_protocol) << ProtocolId;
     return true;
   }
-  if (PDecl->hasDefinition())
+  if (!PDecl->hasDefinition()) {
+    Diag(ProtoLoc, diag::err_atprotocol_protocol) << PDecl;
+    Diag(PDecl->getLocation(), diag::note_entity_declared_at) << PDecl;
+  } else {
     PDecl = PDecl->getDefinition();
+  }
 
   QualType Ty = Context.getObjCProtoType();
   if (Ty.isNull())
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8095,16 +8095,6 @@
     if (RHS.isInvalid())
       return Incompatible;
   }
-
-  Expr *PRE = RHS.get()->IgnoreParenCasts();
-  if (Diagnose && isa<ObjCProtocolExpr>(PRE)) {
-    ObjCProtocolDecl *PDecl = cast<ObjCProtocolExpr>(PRE)->getProtocol();
-    if (PDecl && !PDecl->hasDefinition()) {
-      Diag(PRE->getExprLoc(), diag::warn_atprotocol_protocol) << PDecl;
-      Diag(PDecl->getLocation(), diag::note_entity_declared_at) << PDecl;
-    }
-  }
-
   CastKind Kind;
   Sema::AssignConvertType result =
     CheckAssignmentConstraints(LHSType, RHS, Kind, ConvertRHS);
Index: lib/CodeGen/CGObjCMac.cpp
===================================================================
--- lib/CodeGen/CGObjCMac.cpp
+++ lib/CodeGen/CGObjCMac.cpp
@@ -6783,8 +6783,9 @@
     return Entry;
 
   // Use the protocol definition, if there is one.
-  if (const ObjCProtocolDecl *Def = PD->getDefinition())
-    PD = Def;
+  assert(PD->hasDefinition() &&
+         "emitting protocol metadata without definition");
+  PD = PD->getDefinition();
 
   auto methodLists = ProtocolMethodLists::get(PD);
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -867,8 +867,8 @@
   "protocol has circular dependency">;
 def err_undeclared_protocol : Error<"cannot find protocol declaration for %0">;
 def warn_undef_protocolref : Warning<"cannot find protocol definition for %0">;
-def warn_atprotocol_protocol : Warning<
-  "@protocol is using a forward protocol declaration of %0">, InGroup<AtProtocol>;
+def err_atprotocol_protocol : Error<
+  "@protocol is using a forward protocol declaration of %0">;
 def warn_readonly_property : Warning<
   "attribute 'readonly' of property %0 restricts attribute "
   "'readwrite' of property inherited from %1">,
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -620,7 +620,8 @@
 def SelectorTypeMismatch : DiagGroup<"selector-type-mismatch">;
 def Selector : DiagGroup<"selector", [SelectorTypeMismatch]>;
 def Protocol : DiagGroup<"protocol">;
-def AtProtocol : DiagGroup<"at-protocol">;
+// No longer in use, preserve for backwards compatibility.
+def : DiagGroup<"at-protocol">;
 def PropertyAccessDotSyntax: DiagGroup<"property-access-dot-syntax">;
 def PropertyAttr : DiagGroup<"property-attribute-mismatch">;
 def SuperSubClassMismatch : DiagGroup<"super-class-method-mismatch">;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits