Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Objective-C: The GPBArrays should mark its "values" init param as nullable. #1189

Closed
jcanizales opened this issue Jan 28, 2016 · 14 comments · Fixed by hyperledger/cacti#1190
Closed

Comments

@jcanizales
Copy link
Contributor

Because they're within NS_ASSUME_NONNULL_BEGIN, and the convenience class-level constructors pass NULL in the .m file.

This shows as warnings in XCode 7, which prevent the gRPC podspec from being submitted.

CC: @TeBoring

@TeBoring
Copy link
Contributor

I think something is strange here. NS_ASSUME_NONNULL_BEGIN should be only applied to pointer type. However, in initWithValues:(const int32_t [])values, const int32_t is not a pointer type. At least, when I add nullable to it, compiler warns that it's not a pointer type.

@jcanizales
Copy link
Contributor Author

Changing [] to * there fixed it for me, but I agree that either the compiler is mixing stuff up, or we're missing something.

@TeBoring
Copy link
Contributor

Does that warning block you?

@TeBoring
Copy link
Contributor

Perhaps you can still do pod trunk push --ignore-warnings

@jcanizales
Copy link
Contributor Author

Unless they've changed the policy, not. --allow-warnings used to only work for pushing to private pod repos. For the central repo they impose no-warnings. Still let me check and confirm in a minute.

@jcanizales
Copy link
Contributor Author

Update: Bo's suggestion works now, and this doesn't block gRPC anymore.

@thomasvl
Copy link
Contributor

Which specific Xcode version shows this?

@jcanizales
Copy link
Contributor Author

7.2

@thomasvl
Copy link
Contributor

This warning doesn't trip in the Xcode projects checked in, so it must be something in your project's settings. Can you sort out what is triggering this so we can figure out if there really is something to be done here.

@jcanizales
Copy link
Contributor Author

I had both projects, one that produced the warning (what Cocoapods creates), and one that didn't (the one checked in), side by side on my machine. So same clang version. I looked at the invocation command line, to compare the flags; and I removed those that were equal or that couldn't possible be related. I ended up with:

Produces spurious warning (Cocoapods): clang -arch x86_64 -std=gnu99 -O0 ...
Doesn't produce warning (yours): clang -arch i386 -std=c99 -Os ...

I thought: Surely it's the C standard used, then! GNU99 must say something like "arrays are pointers". So I changed your project to gnu99, and it still didn't produce the warning. I changed Cocoapods' project to c99, and it still produced the warning. As I was left with only the optimization level and the number of bits, and in my heart didn't want any of them to produce this effect, I cursed the gods and gave up. So any help is appreciated.

@dpassage
Copy link

Kibitzing here... Is there Swift code in one project and not the other? NS_ASSUME_NONNULL_BEGIN and company were created to help with importing Objective-C code into Swift projects.

@jcanizales
Copy link
Contributor Author

Actually yes, the project checked in this repo includes GPBSwiftTests.swift, while the projects where I'm seeing the warning don't have Swift code. A clue!!

@thomasvl
Copy link
Contributor

Swift source in the project shouldn't change the warnings generated when compiling ObjC code.

No other warning flag differences between the two projects?

@thomasvl
Copy link
Contributor

According to pod spec lint -

objectivec/GPBArray.m:302:25: warning: null passed to a callee that requires a non-null argument [-Wnonnull]
  return [[[self alloc] initWithValues:NULL count:0] autorelease];

The warning is from -Wnonnull, but if I add that manually to the Xcode project, I don't seem to get the warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants