Skip to content

Commit

Permalink
Add more warnings to for the ObjC runtime build
Browse files Browse the repository at this point in the history
Working on protocolbuffers#1599, specifically:
- Turn on more warnings that the Xcode UI calls out with individual controls.
- Manually add:
  -Wundef
  -Wswitch-enum
- Manually add and then diable in the unittests because of XCTest's headers:
  -Wreserved-id-macro
  -Wdocumentation-unknown-command
- Manually add -Wdirect-ivar-access, but disable it for the unittests and in
  the library code (via #pragmas to suppress it). This is done so proto users
  can enable the warning.
  • Loading branch information
thomasvl committed May 25, 2016
1 parent d089f04 commit c8a440d
Show file tree
Hide file tree
Showing 23 changed files with 269 additions and 32 deletions.
8 changes: 8 additions & 0 deletions objectivec/GPBArray.m
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@

#import "GPBMessage_PackagePrivate.h"

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

// Mutable arrays use an internal buffer that can always hold a multiple of this elements.
#define kChunkSize 16
#define CapacityFromCount(x) (((x / kChunkSize) + 1) * kChunkSize)
Expand Down Expand Up @@ -2532,3 +2538,5 @@ - (void)enumerateObjectsWithOptions:(NSEnumerationOptions)opts
}

@end

#pragma clang diagnostic pop
2 changes: 1 addition & 1 deletion objectivec/GPBBootstrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
// doesn't allow us to forward declare. We work around this one case by
// providing a local definition. The default case has to use NS_ENUM for the
// magic that is Swift bridging of enums.
#if (__cplusplus && __cplusplus < 201103L)
#if (defined(__cplusplus) && __cplusplus && __cplusplus < 201103L)
#define GPB_ENUM(X) enum X : int32_t X; enum X : int32_t
#else
#define GPB_ENUM(X) NS_ENUM(int32_t, X)
Expand Down
2 changes: 1 addition & 1 deletion objectivec/GPBCodedInputStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ NS_ASSUME_NONNULL_BEGIN
///
/// @param message The message to set fields on as they are read.
/// @param extensionRegistry An optional extension registry to use to lookup
/// extensions for @message.
/// extensions for @c message.
- (void)readMessage:(GPBMessage *)message
extensionRegistry:(nullable GPBExtensionRegistry *)extensionRegistry;

Expand Down
8 changes: 8 additions & 0 deletions objectivec/GPBCodedInputStream.m
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,12 @@ - (void)dealloc {
[super dealloc];
}

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

- (int32_t)readTag {
return GPBCodedInputStreamReadTag(&state_);
}
Expand Down Expand Up @@ -496,4 +502,6 @@ - (int64_t)readSInt64 {
return GPBCodedInputStreamReadSInt64(&state_);
}

#pragma clang diagnostic pop

@end
10 changes: 9 additions & 1 deletion objectivec/GPBCodedOutputStream.m
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ static void GPBWriteRawLittleEndian64(GPBOutputBufferState *state,
GPBWriteRawByte(state, (int32_t)(value >> 56) & 0xFF);
}

#if DEBUG && !defined(NS_BLOCK_ASSERTIONS)
#if defined(DEBUG) && DEBUG && !defined(NS_BLOCK_ASSERTIONS)
+ (void)load {
// This test exists to verify that CFStrings with embedded NULLs will work
// for us. If this Assert fails, all code below that depends on
Expand Down Expand Up @@ -203,6 +203,12 @@ + (instancetype)streamWithData:(NSMutableData *)data {
return [[[self alloc] initWithData:data] autorelease];
}

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

- (void)writeDoubleNoTag:(double)value {
GPBWriteRawLittleEndian64(&state_, GPBConvertDoubleToInt64(value));
}
Expand Down Expand Up @@ -981,6 +987,8 @@ - (void)writeRawLittleEndian64:(int64_t)value {
GPBWriteRawLittleEndian64(&state_, value);
}

#pragma clang diagnostic pop

@end

size_t GPBComputeDoubleSizeNoTag(Float64 value) {
Expand Down
2 changes: 1 addition & 1 deletion objectivec/GPBDescriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ typedef NS_ENUM(uint8_t, GPBFieldType) {
@property(nonatomic, readonly, assign) Class msgClass;
@property(nonatomic, readonly) NSString *singletonName;
@property(nonatomic, readonly, strong, nullable) GPBEnumDescriptor *enumDescriptor;
@property(nonatomic, readonly) id defaultValue;
@property(nonatomic, readonly, nullable) id defaultValue;
@end

NS_ASSUME_NONNULL_END
10 changes: 9 additions & 1 deletion objectivec/GPBDescriptor.m
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
#import "GPBWireFormat.h"
#import "GPBMessage_PackagePrivate.h"

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

// The address of this variable is used as a key for obj_getAssociatedObject.
static const char kTextFormatExtraValueKey = 0;

Expand Down Expand Up @@ -803,7 +809,7 @@ - (instancetype)initWithExtensionDescription:
if ((self = [super init])) {
description_ = description;

#if DEBUG
#if defined(DEBUG) && DEBUG
const char *className = description->messageOrGroupClassName;
if (className) {
NSAssert(objc_lookUpClass(className) != Nil,
Expand Down Expand Up @@ -961,3 +967,5 @@ - (NSComparisonResult)compareByFieldNumber:(GPBExtensionDescriptor *)other {
}

@end

#pragma clang diagnostic pop
30 changes: 19 additions & 11 deletions objectivec/GPBDescriptor_PackagePrivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,12 @@ typedef NS_OPTIONS(uint32_t, GPBDescriptorInitializationFlags) {

CF_EXTERN_C_BEGIN

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

GPB_INLINE BOOL GPBFieldIsMapOrArray(GPBFieldDescriptor *field) {
return (field->description_->flags &
(GPBFieldRepeated | GPBFieldMapKeyMask)) != 0;
Expand All @@ -262,6 +268,8 @@ GPB_INLINE uint32_t GPBFieldNumber(GPBFieldDescriptor *field) {
return field->description_->number;
}

#pragma clang diagnostic pop

uint32_t GPBFieldTag(GPBFieldDescriptor *self);

// For repeated fields, alternateWireType is the wireType with the opposite
Expand Down Expand Up @@ -291,23 +299,23 @@ GPB_INLINE BOOL GPBExtensionIsWireFormat(GPBExtensionDescription *description) {
}

// Helper for compile time assets.
#ifndef _GPBCompileAssert
#ifndef GPBInternalCompileAssert
#if __has_feature(c_static_assert) || __has_extension(c_static_assert)
#define _GPBCompileAssert(test, msg) _Static_assert((test), #msg)
#define GPBInternalCompileAssert(test, msg) _Static_assert((test), #msg)
#else
// Pre-Xcode 7 support.
#define _GPBCompileAssertSymbolInner(line, msg) _GPBCompileAssert ## line ## __ ## msg
#define _GPBCompileAssertSymbol(line, msg) _GPBCompileAssertSymbolInner(line, msg)
#define _GPBCompileAssert(test, msg) \
typedef char _GPBCompileAssertSymbol(__LINE__, msg) [ ((test) ? 1 : -1) ]
#define GPBInternalCompileAssertSymbolInner(line, msg) GPBInternalCompileAssert ## line ## __ ## msg
#define GPBInternalCompileAssertSymbol(line, msg) GPBInternalCompileAssertSymbolInner(line, msg)
#define GPBInternalCompileAssert(test, msg) \
typedef char GPBInternalCompileAssertSymbol(__LINE__, msg) [ ((test) ? 1 : -1) ]
#endif // __has_feature(c_static_assert) || __has_extension(c_static_assert)
#endif // _GPBCompileAssert
#endif // GPBInternalCompileAssert

// Sanity check that there isn't padding between the field description
// structures with and without a default.
_GPBCompileAssert(sizeof(GPBMessageFieldDescriptionWithDefault) ==
(sizeof(GPBGenericValue) +
sizeof(GPBMessageFieldDescription)),
DescriptionsWithDefault_different_size_than_expected);
GPBInternalCompileAssert(sizeof(GPBMessageFieldDescriptionWithDefault) ==
(sizeof(GPBGenericValue) +
sizeof(GPBMessageFieldDescription)),
DescriptionsWithDefault_different_size_than_expected);

CF_EXTERN_C_END
11 changes: 11 additions & 0 deletions objectivec/GPBDictionary.m
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@
// directly.
// ------------------------------------------------------------------

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

// Used to include code only visible to specific versions of the static
// analyzer. Useful for wrapping code that only exists to silence the analyzer.
// Determine the values you want to use for BEGIN_APPLE_BUILD_VERSION,
Expand Down Expand Up @@ -484,6 +490,8 @@ void GPBDictionaryReadEntry(id mapDictionary,
key.valueString = [@"" retain];
}
if (GPBDataTypeIsObject(valueDataType) && value.valueString == nil) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wswitch-enum"
switch (valueDataType) {
case GPBDataTypeString:
value.valueString = [@"" retain];
Expand All @@ -505,6 +513,7 @@ void GPBDictionaryReadEntry(id mapDictionary,
// Nothing
break;
}
#pragma clang diagnostic pop
}

if ((keyDataType == GPBDataTypeString) && GPBDataTypeIsObject(valueDataType)) {
Expand Down Expand Up @@ -13553,3 +13562,5 @@ - (void)enumerateKeysAndObjectsWithOptions:(NSEnumerationOptions)opts
}

@end

#pragma clang diagnostic pop
11 changes: 11 additions & 0 deletions objectivec/GPBExtensionInternals.m
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ static id NewSingleValueFromInputStream(GPBExtensionDescriptor *extension,
__attribute__((ns_returns_retained));

GPB_INLINE size_t DataTypeSize(GPBDataType dataType) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wswitch-enum"
switch (dataType) {
case GPBDataTypeBool:
return 1;
Expand All @@ -59,6 +61,7 @@ GPB_INLINE size_t DataTypeSize(GPBDataType dataType) {
default:
return 0;
}
#pragma clang diagnostic pop
}

static size_t ComputePBSerializedSizeNoTagOfObject(GPBDataType dataType, id object) {
Expand Down Expand Up @@ -261,6 +264,12 @@ static void WriteArrayIncludingTagsToCodedOutputStream(
}
}

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

void GPBExtensionMergeFromInputStream(GPBExtensionDescriptor *extension,
BOOL isPackedOnStream,
GPBCodedInputStream *input,
Expand Down Expand Up @@ -378,3 +387,5 @@ static id NewSingleValueFromInputStream(GPBExtensionDescriptor *extension,

return nil;
}

#pragma clang diagnostic pop
8 changes: 8 additions & 0 deletions objectivec/GPBExtensionRegistry.m
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ - (void)dealloc {
[super dealloc];
}

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

- (instancetype)copyWithZone:(NSZone *)zone {
GPBExtensionRegistry *result = [[[self class] allocWithZone:zone] init];
if (result && mutableClassMap_.count) {
Expand Down Expand Up @@ -105,4 +111,6 @@ - (void)addExtensions:(GPBExtensionRegistry *)registry {
}
}

#pragma clang diagnostic pop

@end
22 changes: 15 additions & 7 deletions objectivec/GPBMessage.m
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@
#import "GPBUnknownFieldSet_PackagePrivate.h"
#import "GPBUtilities_PackagePrivate.h"

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

NSString *const GPBMessageErrorDomain =
GPBNSStringifySymbol(GPBMessageErrorDomain);

Expand Down Expand Up @@ -694,7 +700,7 @@ void GPBClearMessageAutocreator(GPBMessage *self) {
return;
}

#if DEBUG && !defined(NS_BLOCK_ASSERTIONS)
#if defined(DEBUG) && DEBUG && !defined(NS_BLOCK_ASSERTIONS)
// Either the autocreator must have its "has" flag set to YES, or it must be
// NO and not equal to ourselves.
BOOL autocreatorHas =
Expand Down Expand Up @@ -1736,7 +1742,7 @@ - (id)getExistingExtension:(GPBExtensionDescriptor *)extension {
}

- (BOOL)hasExtension:(GPBExtensionDescriptor *)extension {
#if DEBUG
#if defined(DEBUG) && DEBUG
CheckExtension(self, extension);
#endif // DEBUG
return nil != [extensionMap_ objectForKey:extension];
Expand Down Expand Up @@ -2621,7 +2627,7 @@ - (BOOL)isEqual:(GPBMessage *)other {
case GPBDataTypeFixed32:
case GPBDataTypeUInt32:
case GPBDataTypeFloat: {
_GPBCompileAssert(sizeof(float) == sizeof(uint32_t), float_not_32_bits);
GPBInternalCompileAssert(sizeof(float) == sizeof(uint32_t), float_not_32_bits);
// These are all 32bit, signed/unsigned doesn't matter for equality.
uint32_t *selfValPtr = (uint32_t *)&selfStorage[fieldOffset];
uint32_t *otherValPtr = (uint32_t *)&otherStorage[fieldOffset];
Expand All @@ -2636,7 +2642,7 @@ - (BOOL)isEqual:(GPBMessage *)other {
case GPBDataTypeFixed64:
case GPBDataTypeUInt64:
case GPBDataTypeDouble: {
_GPBCompileAssert(sizeof(double) == sizeof(uint64_t), double_not_64_bits);
GPBInternalCompileAssert(sizeof(double) == sizeof(uint64_t), double_not_64_bits);
// These are all 64bit, signed/unsigned doesn't matter for equality.
uint64_t *selfValPtr = (uint64_t *)&selfStorage[fieldOffset];
uint64_t *otherValPtr = (uint64_t *)&otherStorage[fieldOffset];
Expand Down Expand Up @@ -2733,7 +2739,7 @@ - (NSUInteger)hash {
case GPBDataTypeFixed32:
case GPBDataTypeUInt32:
case GPBDataTypeFloat: {
_GPBCompileAssert(sizeof(float) == sizeof(uint32_t), float_not_32_bits);
GPBInternalCompileAssert(sizeof(float) == sizeof(uint32_t), float_not_32_bits);
// These are all 32bit, just mix it in.
uint32_t *valPtr = (uint32_t *)&storage[fieldOffset];
result = prime * result + *valPtr;
Expand All @@ -2745,7 +2751,7 @@ - (NSUInteger)hash {
case GPBDataTypeFixed64:
case GPBDataTypeUInt64:
case GPBDataTypeDouble: {
_GPBCompileAssert(sizeof(double) == sizeof(uint64_t), double_not_64_bits);
GPBInternalCompileAssert(sizeof(double) == sizeof(uint64_t), double_not_64_bits);
// These are all 64bit, just mix what fits into an NSUInteger in.
uint64_t *valPtr = (uint64_t *)&storage[fieldOffset];
result = prime * result + (NSUInteger)(*valPtr);
Expand Down Expand Up @@ -2792,7 +2798,7 @@ - (NSString *)description {
return description;
}

#if DEBUG
#if defined(DEBUG) && DEBUG

// Xcode 5.1 added support for custom quick look info.
// https://developer.apple.com/library/ios/documentation/IDEs/Conceptual/CustomClassDisplay_in_QuickLook/CH01-quick_look_for_custom_objects/CH01-quick_look_for_custom_objects.html#//apple_ref/doc/uid/TP40014001-CH2-SW1
Expand Down Expand Up @@ -3182,3 +3188,5 @@ + (BOOL)accessInstanceVariablesDirectly {
}

@end

#pragma clang diagnostic pop
3 changes: 3 additions & 0 deletions objectivec/GPBMessage_PackagePrivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,12 @@ CF_EXTERN_C_BEGIN

// Call this before using the readOnlySemaphore_. This ensures it is created only once.
NS_INLINE void GPBPrepareReadOnlySemaphore(GPBMessage *self) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"
dispatch_once(&self->readOnlySemaphoreCreationOnce_, ^{
self->readOnlySemaphore_ = dispatch_semaphore_create(1);
});
#pragma clang diagnostic pop
}

// Returns a new instance that was automatically created by |autocreator| for
Expand Down
8 changes: 8 additions & 0 deletions objectivec/GPBUnknownField.m
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ - (void)dealloc {
[super dealloc];
}

// Direct access is use for speed, to avoid even internally declaring things
// read/write, etc. The warning is enabled in the project to ensure code calling
// protos can turn on -Wdirect-ivar-access without issues.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

- (id)copyWithZone:(NSZone *)zone {
GPBUnknownField *result =
[[GPBUnknownField allocWithZone:zone] initWithNumber:number_];
Expand Down Expand Up @@ -323,4 +329,6 @@ - (void)addGroup:(GPBUnknownFieldSet *)value {
}
}

#pragma clang diagnostic pop

@end
Loading

0 comments on commit c8a440d

Please sign in to comment.