Skip to content

Fix clang errors#378

Open
davidrothera wants to merge 1 commit intonicklockwood:masterfrom
davidrothera:develop
Open

Fix clang errors#378
davidrothera wants to merge 1 commit intonicklockwood:masterfrom
davidrothera:develop

Conversation

@davidrothera
Copy link
Copy Markdown

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

* Add `resignFirstResponder` calls to the superclass
* Change `self.fields` to `[self fields]` which allows the array to be initiated if it isn't already
@nicklockwood
Copy link
Copy Markdown
Owner

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?

@davidrothera
Copy link
Copy Markdown
Author

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

@nicklockwood
Copy link
Copy Markdown
Owner

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.

@davidrothera
Copy link
Copy Markdown
Author

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

@nicklockwood
Copy link
Copy Markdown
Owner

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]

@davidrothera
Copy link
Copy Markdown
Author

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

@nicklockwood
Copy link
Copy Markdown
Owner

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

@davidrothera
Copy link
Copy Markdown
Author

Quite possibly, clang isn't the smartest ;)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants