FXForms icon indicating copy to clipboard operation
FXForms copied to clipboard

Fix clang errors

Open davidrothera opened this issue 9 years ago • 8 comments

Changes

  • Add resignFirstResponder calls to the superclass
  • Change self.fields to [self fields] which allows the array to be initiated if it isn't already

Fixes

Fixes #365

Testing

  • Ran unit tests, no failures
  • Ran clang analyze again, no issues

davidrothera avatar Jun 03 '15 16:06 davidrothera

That doesn't make a whole lot of sense. self.fields is functionally equivalent to [self fields]. Can you explain what effect you believe this change had on the behavior?

nicklockwood avatar Jun 16 '15 17:06 nicklockwood

The basis behind my change was that calling self.fields accesses the instance variable which could be nil however using the [self fields] method[1] will instantiate a non-existing instance variable.

[1] https://github.com/davidrothera/FXForms/blob/develop/FXForms/FXForms.m#L1732

davidrothera avatar Jun 16 '15 17:06 davidrothera

But self.fields doesn't access the instance variable. It calls the - (NSMutableArray *)fields method. To access the instance variable, you'd have to write self->_fields.

Try putting a breakpoint inside - (NSMutableArray *)fields and you can verify that it's called when you access self.fields.

I think perhaps the issue that you're referring to is "Argument to 'NSMutableArray' method 'addObject;:' cannot be nil"? In which case the issue it's complaining about is that the field value being added to the array is nil, not the array itself.

I'll fix this in the next update.

nicklockwood avatar Jun 16 '15 20:06 nicklockwood

Yeah I think you are right, its strange though as making the change in the PR resolved the clang error and retained all functionality.

davidrothera avatar Jun 16 '15 20:06 davidrothera

Hmm, I can't actually replicate a clang warning on the line you fixed. The warning I'm seeing is on the line a few below that: [self.fields addObject:field]

nicklockwood avatar Jun 16 '15 20:06 nicklockwood

Yes, the error was on that line and the change I made resolved that one lower down.

davidrothera avatar Jun 16 '15 20:06 davidrothera

I think maybe the change above just confused the analyzer so that it couldn't parse the logic below anymore ¯_(ツ)_/¯

nicklockwood avatar Jun 16 '15 20:06 nicklockwood

Quite possibly, clang isn't the smartest ;)

davidrothera avatar Jun 16 '15 20:06 davidrothera