From dbe2b9e976d50ce7971df32dc9a3122525dad3ce Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 15 Jun 2016 20:23:04 -0700 Subject: [PATCH 1/5] Enable treating warnings as errors in objc tests --- src/objective-c/tests/GRPCClientTests.m | 16 ++++++++-------- src/objective-c/tests/InteropTests.m | 10 ++++++---- .../tests/Tests.xcodeproj/project.pbxproj | 4 ++++ 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 9a8d4253241..2eca7bf5498 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -48,9 +48,9 @@ static NSString * const kPackage = @"grpc.testing"; static NSString * const kService = @"TestService"; static NSString * const kRemoteSSLHost = @"grpc-test.sandbox.googleapis.com"; -static ProtoMethod *kInexistentMethod; -static ProtoMethod *kEmptyCallMethod; -static ProtoMethod *kUnaryCallMethod; +static GRPCProtoMethod *kInexistentMethod; +static GRPCProtoMethod *kEmptyCallMethod; +static GRPCProtoMethod *kUnaryCallMethod; /** Observer class for testing that responseMetadata is KVO-compliant */ @interface PassthroughObserver : NSObject @@ -109,13 +109,13 @@ static ProtoMethod *kUnaryCallMethod; [GRPCCall useInsecureConnectionsForHost:kHostAddress]; // This method isn't implemented by the remote server. - kInexistentMethod = [[ProtoMethod alloc] initWithPackage:kPackage + kInexistentMethod = [[GRPCProtoMethod alloc] initWithPackage:kPackage service:kService method:@"Inexistent"]; - kEmptyCallMethod = [[ProtoMethod alloc] initWithPackage:kPackage + kEmptyCallMethod = [[GRPCProtoMethod alloc] initWithPackage:kPackage service:kService method:@"EmptyCall"]; - kUnaryCallMethod = [[ProtoMethod alloc] initWithPackage:kPackage + kUnaryCallMethod = [[GRPCProtoMethod alloc] initWithPackage:kPackage service:kService method:@"UnaryCall"]; } @@ -303,7 +303,7 @@ static ProtoMethod *kUnaryCallMethod; // Try to set parameters to nil for GRPCCall. This should cause an exception @try { - GRPCCall *call = [[GRPCCall alloc] initWithHost:nil + GRPCCall *call __unused = [[GRPCCall alloc] initWithHost:nil path:nil requestsWriter:nil]; XCTFail(@"Did not receive an exception when parameters are nil"); @@ -316,7 +316,7 @@ static ProtoMethod *kUnaryCallMethod; GRXWriter *requestsWriter = [GRXWriter emptyWriter]; [requestsWriter finishWithError:nil]; @try { - GRPCCall *call = [[GRPCCall alloc] initWithHost:kHostAddress + GRPCCall *call __unused = [[GRPCCall alloc] initWithHost:kHostAddress path:kUnaryCallMethod.HTTPPath requestsWriter:requestsWriter]; XCTFail(@"Did not receive an exception when GRXWriter has incorrect state."); diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index 781c500f73e..60a83259fa6 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -58,7 +58,7 @@ requestedResponseSize:(NSNumber *)responseSize { RMTStreamingOutputCallRequest *request = [self message]; RMTResponseParameters *parameters = [RMTResponseParameters message]; - parameters.size = responseSize.integerValue; + parameters.size = (int)responseSize.integerValue; [request.responseParametersArray addObject:parameters]; request.payload.body = [NSMutableData dataWithLength:payloadSize.unsignedIntegerValue]; return request; @@ -80,7 +80,9 @@ #pragma mark Tests +#ifdef GRPC_COMPILE_WITH_CRONET static cronet_engine *cronetEngine = NULL; +#endif @implementation InteropTests { RMTTestService *_service; @@ -186,7 +188,7 @@ static cronet_engine *cronetEngine = NULL; RMTStreamingOutputCallRequest *request = [RMTStreamingOutputCallRequest message]; for (NSNumber *size in expectedSizes) { RMTResponseParameters *parameters = [RMTResponseParameters message]; - parameters.size = [size integerValue]; + parameters.size = (int)[size integerValue]; [request.responseParametersArray addObject:parameters]; } @@ -282,7 +284,7 @@ static cronet_engine *cronetEngine = NULL; // A buffered pipe to which we never write any value acts as a writer that just hangs. GRXBufferedPipe *requestsBuffer = [[GRXBufferedPipe alloc] init]; - ProtoRPC *call = [_service RPCToStreamingInputCallWithRequestsWriter:requestsBuffer + GRPCProtoCall *call = [_service RPCToStreamingInputCallWithRequestsWriter:requestsBuffer handler:^(RMTStreamingInputCallResponse *response, NSError *error) { XCTAssertEqual(error.code, GRPC_STATUS_CANCELLED); @@ -313,7 +315,7 @@ static cronet_engine *cronetEngine = NULL; [requestsBuffer writeValue:request]; - __block ProtoRPC *call = + __block GRPCProtoCall *call = [_service RPCToFullDuplexCallWithRequestsWriter:requestsBuffer eventHandler:^(BOOL done, RMTStreamingOutputCallResponse *response, diff --git a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj index b0429617c01..59a6af2e11e 100644 --- a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj +++ b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj @@ -823,6 +823,7 @@ "$(inherited)", ); GCC_SYMBOLS_PRIVATE_EXTERN = NO; + GCC_TREAT_WARNINGS_AS_ERRORS = YES; GCC_WARN_64_TO_32_BIT_CONVERSION = YES; GCC_WARN_ABOUT_RETURN_TYPE = YES_ERROR; GCC_WARN_UNDECLARED_SELECTOR = YES; @@ -859,6 +860,7 @@ ENABLE_STRICT_OBJC_MSGSEND = YES; GCC_C_LANGUAGE_STANDARD = gnu99; GCC_NO_COMMON_BLOCKS = YES; + GCC_TREAT_WARNINGS_AS_ERRORS = YES; GCC_WARN_64_TO_32_BIT_CONVERSION = YES; GCC_WARN_ABOUT_RETURN_TYPE = YES_ERROR; GCC_WARN_UNDECLARED_SELECTOR = YES; @@ -875,6 +877,7 @@ 635697DC1B14FC11007A7283 /* Debug */ = { isa = XCBuildConfiguration; buildSettings = { + GCC_TREAT_WARNINGS_AS_ERRORS = YES; PRODUCT_NAME = "$(TARGET_NAME)"; SKIP_INSTALL = YES; }; @@ -883,6 +886,7 @@ 635697DD1B14FC11007A7283 /* Release */ = { isa = XCBuildConfiguration; buildSettings = { + GCC_TREAT_WARNINGS_AS_ERRORS = YES; PRODUCT_NAME = "$(TARGET_NAME)"; SKIP_INSTALL = YES; }; From 36a58a7928e6a10b44b6cffdc118337ef7e0b8c1 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Mon, 20 Jun 2016 14:01:07 -0700 Subject: [PATCH 2/5] Enable treating warnings as errors for target gRPC --- .../GRPCClient/GRPCCall+ChannelCredentials.h | 4 ++-- src/objective-c/ProtoRPC/ProtoMethod.h | 3 +++ src/objective-c/ProtoRPC/ProtoRPC.h | 3 +++ src/objective-c/ProtoRPC/ProtoService.h | 3 +++ src/objective-c/ProtoRPC/ProtoService.m | 6 +++--- src/objective-c/tests/Podfile | 21 ++++++++++++++++--- 6 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall+ChannelCredentials.h b/src/objective-c/GRPCClient/GRPCCall+ChannelCredentials.h index 343dd48a14e..ac2a37d75f2 100644 --- a/src/objective-c/GRPCClient/GRPCCall+ChannelCredentials.h +++ b/src/objective-c/GRPCClient/GRPCCall+ChannelCredentials.h @@ -41,7 +41,7 @@ */ + (BOOL)setTLSPEMRootCerts:(nullable NSString *)pemRootCert forHost:(nonnull NSString *)host - error:(NSError **)errorPtr; + error:(NSError * _Nullable * _Nullable)errorPtr; /** * Configures @c host with TLS/SSL Client Credentials and optionally trusted root Certificate * Authorities. If @c pemRootCerts is nil, the default CA Certificates bundled with gRPC will be @@ -51,6 +51,6 @@ withPrivateKey:(nullable NSString *)pemPrivateKey withCertChain:(nullable NSString *)pemCertChain forHost:(nonnull NSString *)host - error:(NSError **)errorPtr; + error:(NSError * _Nullable * _Nullable)errorPtr; @end diff --git a/src/objective-c/ProtoRPC/ProtoMethod.h b/src/objective-c/ProtoRPC/ProtoMethod.h index f9fdbb35ffd..ae3a2723fc4 100644 --- a/src/objective-c/ProtoRPC/ProtoMethod.h +++ b/src/objective-c/ProtoRPC/ProtoMethod.h @@ -54,6 +54,9 @@ __attribute__((deprecated("Please use GRPCProtoMethod."))) * This subclass is empty now. Eventually we'll remove ProtoMethod class * to avoid potential naming conflict */ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" @interface GRPCProtoMethod : ProtoMethod +#pragma clang diagnostic pop @end diff --git a/src/objective-c/ProtoRPC/ProtoRPC.h b/src/objective-c/ProtoRPC/ProtoRPC.h index 5f91f6bce1e..04fc1e45f16 100644 --- a/src/objective-c/ProtoRPC/ProtoRPC.h +++ b/src/objective-c/ProtoRPC/ProtoRPC.h @@ -56,6 +56,9 @@ __attribute__((deprecated("Please use GRPCProtoCall."))) * This subclass is empty now. Eventually we'll remove ProtoRPC class * to avoid potential naming conflict */ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" @interface GRPCProtoCall : ProtoRPC +#pragma clang diagnostic pop @end diff --git a/src/objective-c/ProtoRPC/ProtoService.h b/src/objective-c/ProtoRPC/ProtoService.h index 87d06e1ae59..7faae1b49c9 100644 --- a/src/objective-c/ProtoRPC/ProtoService.h +++ b/src/objective-c/ProtoRPC/ProtoService.h @@ -55,6 +55,9 @@ __attribute__((deprecated("Please use GRPCProtoService."))) * This subclass is empty now. Eventually we'll remove ProtoService class * to avoid potential naming conflict */ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" @interface GRPCProtoService : ProtoService +#pragma clang diagnostic pop @end diff --git a/src/objective-c/ProtoRPC/ProtoService.m b/src/objective-c/ProtoRPC/ProtoService.m index 597c3cf0fed..4a14570d818 100644 --- a/src/objective-c/ProtoRPC/ProtoService.m +++ b/src/objective-c/ProtoRPC/ProtoService.m @@ -65,14 +65,14 @@ return self; } -- (ProtoRPC *)RPCToMethod:(NSString *)method +- (GRPCProtoCall *)RPCToMethod:(NSString *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id)responsesWriteable { - ProtoMethod *methodName = [[ProtoMethod alloc] initWithPackage:_packageName + GRPCProtoMethod *methodName = [[GRPCProtoMethod alloc] initWithPackage:_packageName service:_serviceName method:method]; - return [[ProtoRPC alloc] initWithHost:_host + return [[GRPCProtoCall alloc] initWithHost:_host method:methodName requestsWriter:requestsWriter responseClass:responseClass diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index 508641d681a..53edf8c890a 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -1,9 +1,9 @@ source 'https://github.com/CocoaPods/Specs.git' platform :ios, '8.0' -pod 'Protobuf', :path => "../../../third_party/protobuf" -pod 'BoringSSL', :podspec => ".." -pod 'CronetFramework', :podspec => ".." +pod 'Protobuf', :path => "../../../third_party/protobuf", :inhibit_warnings => true +pod 'BoringSSL', :podspec => "..", :inhibit_warnings => true +pod 'CronetFramework', :podspec => "..", :inhibit_warnings => true pod 'gRPC', :path => "../../.." pod 'RemoteTest', :path => "RemoteTestClient" @@ -30,3 +30,18 @@ end target 'InteropTestsLocalCleartext' do end + +post_install do |installer| + installer.pods_project.targets.each do |target| + target.build_configurations.each do |config| + config.build_settings['GCC_TREAT_WARNINGS_AS_ERRORS'] = 'YES' + end + if target.name == 'gRPC' + target.build_configurations.each do |config| + # GPR_UNREACHABLE_CODE causes "Control may reach end of non-void + # function" warning + config.build_settings['GCC_WARN_ABOUT_RETURN_TYPE'] = 'NO' + end + end + end +end From aa338326ed399832394078282664c6bcf66d11e7 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Mon, 20 Jun 2016 16:47:40 -0700 Subject: [PATCH 3/5] Revert ProtoService.m, add an exception for its incompatible-pointer-types warning --- src/objective-c/ProtoRPC/ProtoService.m | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/objective-c/ProtoRPC/ProtoService.m b/src/objective-c/ProtoRPC/ProtoService.m index 4a14570d818..cd9bc7aeac4 100644 --- a/src/objective-c/ProtoRPC/ProtoService.m +++ b/src/objective-c/ProtoRPC/ProtoService.m @@ -65,19 +65,22 @@ return self; } -- (GRPCProtoCall *)RPCToMethod:(NSString *)method +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wincompatible-pointer-types" +- (ProtoRPC *)RPCToMethod:(NSString *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id)responsesWriteable { - GRPCProtoMethod *methodName = [[GRPCProtoMethod alloc] initWithPackage:_packageName + ProtoMethod *methodName = [[ProtoMethod alloc] initWithPackage:_packageName service:_serviceName method:method]; - return [[GRPCProtoCall alloc] initWithHost:_host + return [[ProtoRPC alloc] initWithHost:_host method:methodName requestsWriter:requestsWriter responseClass:responseClass responsesWriteable:responsesWriteable]; } +#pragma clang diagnostic pop @end @implementation GRPCProtoService From d5fd7ddc70e658a0364bd2e43c8a486a25db267c Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Tue, 21 Jun 2016 11:13:23 -0700 Subject: [PATCH 4/5] Addressed review comments Removed the silencing for incompatible-pointer-types Removed unused objects Fixed format issues --- src/objective-c/ProtoRPC/ProtoService.m | 9 +++------ src/objective-c/tests/GRPCClientTests.m | 24 ++++++++++++------------ src/objective-c/tests/Podfile | 3 ++- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/objective-c/ProtoRPC/ProtoService.m b/src/objective-c/ProtoRPC/ProtoService.m index cd9bc7aeac4..97401908519 100644 --- a/src/objective-c/ProtoRPC/ProtoService.m +++ b/src/objective-c/ProtoRPC/ProtoService.m @@ -65,22 +65,19 @@ return self; } -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wincompatible-pointer-types" - (ProtoRPC *)RPCToMethod:(NSString *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id)responsesWriteable { - ProtoMethod *methodName = [[ProtoMethod alloc] initWithPackage:_packageName - service:_serviceName - method:method]; + GRPCProtoMethod *methodName = [[GRPCProtoMethod alloc] initWithPackage:_packageName + service:_serviceName + method:method]; return [[ProtoRPC alloc] initWithHost:_host method:methodName requestsWriter:requestsWriter responseClass:responseClass responsesWriteable:responsesWriteable]; } -#pragma clang diagnostic pop @end @implementation GRPCProtoService diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 2eca7bf5498..1167a715bb9 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -110,14 +110,14 @@ static GRPCProtoMethod *kUnaryCallMethod; // This method isn't implemented by the remote server. kInexistentMethod = [[GRPCProtoMethod alloc] initWithPackage:kPackage - service:kService - method:@"Inexistent"]; + service:kService + method:@"Inexistent"]; kEmptyCallMethod = [[GRPCProtoMethod alloc] initWithPackage:kPackage - service:kService - method:@"EmptyCall"]; + service:kService + method:@"EmptyCall"]; kUnaryCallMethod = [[GRPCProtoMethod alloc] initWithPackage:kPackage - service:kService - method:@"UnaryCall"]; + service:kService + method:@"UnaryCall"]; } - (void)testConnectionToRemoteServer { @@ -303,9 +303,9 @@ static GRPCProtoMethod *kUnaryCallMethod; // Try to set parameters to nil for GRPCCall. This should cause an exception @try { - GRPCCall *call __unused = [[GRPCCall alloc] initWithHost:nil - path:nil - requestsWriter:nil]; + (void)[[GRPCCall alloc] initWithHost:nil + path:nil + requestsWriter:nil]; XCTFail(@"Did not receive an exception when parameters are nil"); } @catch(NSException *theException) { NSLog(@"Received exception as expected: %@", theException.name); @@ -316,9 +316,9 @@ static GRPCProtoMethod *kUnaryCallMethod; GRXWriter *requestsWriter = [GRXWriter emptyWriter]; [requestsWriter finishWithError:nil]; @try { - GRPCCall *call __unused = [[GRPCCall alloc] initWithHost:kHostAddress - path:kUnaryCallMethod.HTTPPath - requestsWriter:requestsWriter]; + (void)[[GRPCCall alloc] initWithHost:kHostAddress + path:kUnaryCallMethod.HTTPPath + requestsWriter:requestsWriter]; XCTFail(@"Did not receive an exception when GRXWriter has incorrect state."); } @catch(NSException *theException) { NSLog(@"Received exception as expected: %@", theException.name); diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index d51b18cc34d..6d5f94cbda1 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -6,7 +6,7 @@ install! 'cocoapods', :deterministic_uuids => false def shared_pods pod 'Protobuf', :path => "../../../third_party/protobuf", :inhibit_warnings => true pod 'BoringSSL', :podspec => "..", :inhibit_warnings => true - pod 'CronetFramework', :podspec => "..", :inhibit_warnings => true + pod 'CronetFramework', :podspec => ".." pod 'gRPC', :path => "../../.." pod 'RemoteTest', :path => "RemoteTestClient" end @@ -42,6 +42,7 @@ post_install do |installer| end if target.name == 'gRPC' target.build_configurations.each do |config| + # TODO(zyc) Remove this setting after the issue is resolved # GPR_UNREACHABLE_CODE causes "Control may reach end of non-void # function" warning config.build_settings['GCC_WARN_ABOUT_RETURN_TYPE'] = 'NO' From 8d9e83806d24ad5e1a47bb705d88816bfc8b4864 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Tue, 21 Jun 2016 11:21:48 -0700 Subject: [PATCH 5/5] Fixed format issues --- src/objective-c/tests/InteropTests.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index 60a83259fa6..15ce120c551 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -285,8 +285,8 @@ static cronet_engine *cronetEngine = NULL; GRXBufferedPipe *requestsBuffer = [[GRXBufferedPipe alloc] init]; GRPCProtoCall *call = [_service RPCToStreamingInputCallWithRequestsWriter:requestsBuffer - handler:^(RMTStreamingInputCallResponse *response, - NSError *error) { + handler:^(RMTStreamingInputCallResponse *response, + NSError *error) { XCTAssertEqual(error.code, GRPC_STATUS_CANCELLED); [expectation fulfill]; }];