> On Jan. 27, 2017, 3 p.m., Jason Lowe-Power wrote:
> > Oops. I should have included this in my description of
> > http://reviews.gem5.org/r/3792/. I think another solution is to include
> > Python.h in src/python/swig/pyevent.hh. See my change in
> > http://reviews.gem5.org/r/3792/diff/1/#33.
> >
> > If my fix does work, I think it's better to just include Python.h there
> > instead of changing the style.
>
> Andreas Sandberg wrote:
> I'd like to avoid including Python.h in any header files for dependency
> reasons. Another reason for including it before any other header is that it
> solves this entire class of issues (the flip side is that we wouldn't always
> spot headers that requires it).
>
> We actually used to require the Python header to be first in the files
> that required it, but it got lost in a a style update a while back.
>
> Jason Lowe-Power wrote:
> Could you explain more about why you don't want to include Python.h in
> pyevent.hh? It seems to me that since this file requires things declared in
> Python.h (e.g., PyObject) that it should be included here. Otherwise, we have
> to remember that anytime we include python/swig/pyevent.hh we first must
> include Python.h.
>
> Andreas Sandberg wrote:
> Sorry, you're absolutely right. It should definitely be included in
> pyevent.hh. Does that solve the rest of the Python issues? If so, do we
> need/want this change?
>
> Jason Lowe-Power wrote:
> I *think* it solves all of the issues. I was able to use
> http://reviews.gem5.org/r/3792/ and http://reviews.gem5.org/r/3779/ to
> resolve all of the build issues on Arch Linux.
>
> I'm not opposed to pushing this to change the style back, if we think
> that makes sense, too. But I think it's somewhat orthogonal to the build
> errors.
I had a look at the Python manual, this is what it states:
Note Since Python may define some pre-processor definitions which affect
the standard headers on some systems, you must include Python.h before any
standard headers are included.
Seems like we should include Python.h first after all.
- Andreas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3794/#review9331
-----------------------------------------------------------
On Jan. 27, 2017, 1:39 p.m., Andreas Sandberg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3794/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2017, 1:39 p.m.)
>
>
> Review request for Default.
>
>
> Repository: gem5
>
>
> Description
> -------
>
> Changeset 11803:1dba09aeeefd
> ---------------------------
> style: Force Python.h to be included before main header
>
> Python's header files set various compiler macros (e.g.,
> _XOPEN_SOURCE) unconditionally. This triggers preprocessor warnings
> that end up being treated as errors. The style guide used to mandate
> that Python headers are included before any other header. This
> requirement was changed to always include a source file's main header
> first, which ended up triggering these errors.
>
> This change updates the style checker to always include Python.h
> before the main header file.
>
> Change-Id: Id6a4f7fc64a336a8fd26691a0ca682abeb1d1579
> Signed-off-by: Andreas Sandberg <[email protected]>
> Reviewed-by: Nikos Nikoleris <[email protected]>
>
>
> Diffs
> -----
>
> src/python/swig/pyevent.cc be62996c95d1
> src/sim/init.cc be62996c95d1
> src/sim/py_interact.cc be62996c95d1
> util/style/sort_includes.py be62996c95d1
>
> Diff: http://reviews.gem5.org/r/3794/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andreas Sandberg
>
>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
