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

Make functions static, ensure declarations match headers #610

Merged
merged 1 commit into from
Jul 13, 2016
Merged

Make functions static, ensure declarations match headers #610

merged 1 commit into from
Jul 13, 2016

Conversation

ideasman42
Copy link
Collaborator

@ideasman42 ideasman42 commented Jul 9, 2016

This patch used -Wmissing-declarations to show functions and symbols that had no declarations, and either make them static or declare in headers.

This helps avoid possibility that declarations and functions get out of sync. (which can cause hard to track down crashes which may only happen on some configurations)
And ensures all source files reference headers correctly.

This ensures functions declared in extern "C", do so in the header too.
An error found in calligraph.h while writing this patch.

It helps with refactoring code, when its obvious that functions are only ever used locally, theres no need to double check if they're called from other files, or forget to and cause bugs.

The compiler may also do a better job optimizing the code it knows isn't used elsewhere (inline static calls, fold branches when called with constant arguments).

This has been applied to toonzlib, to avoid making very large global changes.
If accepted, other modules could have similar changes made and -Wmissing-declarations added to CMake.

This patch used -Wmissing-declarations warning
to show functions and symbols that had no declarations, and either:

- Make static
- Add to header

This helps avoid possability that declarations and functions get out of sync.
And ensures all source files reference headers correctly.

It also makes sure functions defined with extern "C",
have this defined in the header. An error found in calligraph.h while writing this patch.

This has been applied to toonzlib, to avoid making very large global changes.
If accepted, -Wmissing-declarations warning could be added to CMake.
@skitaoka
Copy link
Member

Jenkins, test this.

@skitaoka
Copy link
Member

LGTM

@skitaoka skitaoka merged commit b3bd842 into opentoonz:master Jul 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants