Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix compiler warnings #1725

Merged
merged 1 commit into from

4 participants

@dmcdougall
Collaborator

Some of the compiler warnings started to bug me, so I tried to fix them. They're usually harmless, but sometimes they are not, as I will explain.

Currently, these are the warnings that are thrown during a python2.7 build of matplotlib (clang on a mac is used here)

After this patch is applied, these are the warnings that are still left. The first four are due to a 'bug' in the python API; treating string literals as char * rather than const char *. There is a patch upstream for it, but I'm not sure if it will be back-ported to python 2.7.4.

The last bunch of Tcl/Tk warnings are because the compiler is being handed both a -c flag and a -framework T{cl,k} flag, which are meant solely for the compilation and linking steps, respectively. To fix them, one would need to separate out the compile and link steps, but I don't know how to do that with distutils, and they only appear on the mac. Linux and windows compilers do not support the -framework flag.

I have tested this patch and I get a successful build with both Apple's builds of gcc and clang. All the tests pass locally with python 2.7. It would be nice to have these tested with python3, and a linux flavour of compiler.

I also fixed a typo in a comment.

@dmcdougall
Collaborator

I didn't target 1.2.x on purpose. These don't fix bugs, and I didn't want them in 1.2.x 'just in case' they broke something I didn't think about.

@dmcdougall dmcdougall commented on the diff
src/_backend_agg.cpp
@@ -2048,7 +2055,9 @@ class QuadMeshGenerator
{
if (fwrite(pixBuffer, 1, NUMBYTES, fp) != NUMBYTES)
{
- npy_PyFile_DupClose(py_file, fp);
+ if (npy_PyFile_DupClose(py_file, fp)) {
+ throw Py::RuntimeError("Error closing dupe file handle");
+ }
@dmcdougall Collaborator

All this block does is make use of the value npy_PyFile_DupClose returns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dmcdougall dmcdougall commented on the diff
src/_image.cpp
@@ -406,10 +406,6 @@
typedef agg::span_allocator<agg::rgba8> span_alloc_type;
span_alloc_type sa;
- agg::rgba8 background(agg::rgba8(int(255*bg.r),
- int(255*bg.g),
- int(255*bg.b),
- int(255*bg.a)));
@dmcdougall Collaborator

The compiler told me this was unused, so I removed it.

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

debian sid, gcc 4.7.2, python 2.7.3

warnings on master https://gist.github.com/4683894

Warnings on this branch https://gist.github.com/4683594

@pelson pelson commented on the diff
src/_backend_agg.cpp
@@ -39,6 +39,13 @@
#include "numpy/arrayobject.h"
#include "agg_py_transforms.h"
+
+// This C function resides in npy_3kcompat.h and is unused by
+// matplotlib. This is here so that the compiler doesn't complain
+// that it is unused.
+extern "C" {
+__attribute__((unused)) static void simple_capsule_dtor(void *ptr);
@pelson Collaborator
pelson added a note

Personally, I have absolutely no idea what this does (black magic :smile:). Could somebody who does give it the :+1: please?

@mdboom Owner
mdboom added a note

Seems fine, except I wish there were a cleaner way to hide this warning... Don't know what that would be.

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

Other than my lack of understanding which I have pointed out, I'd be happy enough to merge this. Your right about targetting v1.3.x - I would be very uncomfortable with this kind of change slipping into v1.2.x as we could easily be introducing a subtle regression...

@mdboom
Owner

Looks fine -- but I may have some further changes shortly for some more warnings I'm still getting on my platform.

@dmcdougall
Collaborator

@mdboom Go for it! :)

@mdboom
Owner

I'm not seeing the compiler warnings I was seeing before -- perhaps I hadn't enough coffee that day. I think this is good to merge...

@mdboom mdboom merged commit 091c149 into matplotlib:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 31, 2013
  1. @dmcdougall

    Fix compiler warnings

    dmcdougall authored
This page is out of date. Refresh to see the latest.
View
2  setupext.py
@@ -1030,7 +1030,7 @@ def add_tk_flags(module):
if not exists(join(F, fw + '.framework')):
break
else:
- # ok, F is now directory with both frameworks. Continure
+ # ok, F is now directory with both frameworks. Continue
# building
tk_framework_found = 1
break
View
15 src/_backend_agg.cpp
@@ -39,6 +39,13 @@
#include "numpy/arrayobject.h"
#include "agg_py_transforms.h"
+
+// This C function resides in npy_3kcompat.h and is unused by
+// matplotlib. This is here so that the compiler doesn't complain
+// that it is unused.
+extern "C" {
+__attribute__((unused)) static void simple_capsule_dtor(void *ptr);
@pelson Collaborator
pelson added a note

Personally, I have absolutely no idea what this does (black magic :smile:). Could somebody who does give it the :+1: please?

@mdboom Owner
mdboom added a note

Seems fine, except I wish there were a cleaner way to hide this warning... Don't know what that would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+}
#include "file_compat.h"
#ifndef M_PI
@@ -2048,7 +2055,9 @@ RendererAgg::write_rgba(const Py::Tuple& args)
{
if (fwrite(pixBuffer, 1, NUMBYTES, fp) != NUMBYTES)
{
- npy_PyFile_DupClose(py_file, fp);
+ if (npy_PyFile_DupClose(py_file, fp)) {
+ throw Py::RuntimeError("Error closing dupe file handle");
+ }
@dmcdougall Collaborator

All this block does is make use of the value npy_PyFile_DupClose returns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
if (close_file) {
npy_PyFile_CloseFile(py_file);
@@ -2058,7 +2067,9 @@ RendererAgg::write_rgba(const Py::Tuple& args)
throw Py::RuntimeError("Error writing to file");
}
- npy_PyFile_DupClose(py_file, fp);
+ if (npy_PyFile_DupClose(py_file, fp)) {
+ throw Py::RuntimeError("Error closing dupe file handle");
+ }
if (close_file) {
npy_PyFile_CloseFile(py_file);
View
4 src/_image.cpp
@@ -406,10 +406,6 @@ Image::resize(const Py::Tuple& args, const Py::Dict& kwargs)
typedef agg::span_allocator<agg::rgba8> span_alloc_type;
span_alloc_type sa;
- agg::rgba8 background(agg::rgba8(int(255*bg.r),
- int(255*bg.g),
- int(255*bg.b),
- int(255*bg.a)));
@dmcdougall Collaborator

The compiler told me this was unused, so I removed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
// the image path
agg::path_storage path;
View
19 src/_png.cpp
@@ -27,6 +27,13 @@
#include "CXX/Extensions.hxx"
#include "numpy/arrayobject.h"
#include "mplutils.h"
+
+// This C function resides in npy_3kcompat.h and is unused by
+// matplotlib. This is here so that the compiler doesn't complain
+// that it is unused.
+extern "C" {
+__attribute__((unused)) static void simple_capsule_dtor(void *ptr);
+}
#include "file_compat.h"
// As reported in [3082058] build _png.so on aix
@@ -239,7 +246,9 @@ Py::Object _png_module::write_png(const Py::Tuple& args)
if (close_dup_file)
{
- npy_PyFile_DupClose(py_file, fp);
+ if (npy_PyFile_DupClose(py_file, fp)) {
+ throw Py::RuntimeError("Error closing dupe file handle");
+ }
}
if (close_file)
@@ -258,7 +267,9 @@ Py::Object _png_module::write_png(const Py::Tuple& args)
delete [] row_pointers;
if (close_dup_file)
{
- npy_PyFile_DupClose(py_file, fp);
+ if (npy_PyFile_DupClose(py_file, fp)) {
+ throw Py::RuntimeError("Error closing dupe file handle");
+ }
}
if (close_file)
@@ -569,7 +580,9 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result,
#endif
if (close_dup_file)
{
- npy_PyFile_DupClose(py_file, fp);
+ if (npy_PyFile_DupClose(py_file, fp)) {
+ throw Py::RuntimeError("Error closing dupe file handle");
+ }
}
if (close_file)
Something went wrong with that request. Please try again.