> 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?
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. - Jason ----------------------------------------------------------- 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
