| Message ID | 1455031511-23684-4-git-send-email-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 9 February 2016 at 15:25, Peter Maydell <peter.maydell@linaro.org> wrote: > Rearrange include directives so that we include osdep.h first. > This has to be done manually because clean-includes doesn't > handle C++. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > disas/arm-a64.cc | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/disas/arm-a64.cc b/disas/arm-a64.cc > index d4d46d5..9280950 100644 > --- a/disas/arm-a64.cc > +++ b/disas/arm-a64.cc > @@ -17,12 +17,13 @@ > * along with this program. If not, see <http://www.gnu.org/licenses/>. > */ > > -#include "vixl/a64/disasm-a64.h" > - > extern "C" { > +#include "qemu/osdep.h" > #include "disas/bfd.h" > } > > +#include "vixl/a64/disasm-a64.h" > + > using namespace vixl; > > static Decoder *vixl_decoder = NULL; So this patch doesn't build on the old mingw32 compiler. I think this is because this compiler is not C++11, and so its <stdint.h> doesn't provide various macros for C++ unless __STDC_CONSTANT_MACROS, __STDC_LIMIT_MACROS and __STDC_FORMAT_MACROS are defined before the first inclusion of <stdint.h>. libvixl's globals.h defines these constants, but this only works if globals.h is first-include, so making osdep.h first-include then results in stdint.h being included before globals.h has a chance to set the defines. What's the best way to deal with this? I can see a couple of options: (1) as a special case, for this file include disasm-a64.h before osdep.h (2) as a special case, for this file manually define the __STDC_* before including osdep.h (3) make osdep.h itself define the __STDC_* constants so it works with C++-before-C++11 as well as with C and with C++11 I think I prefer (3) (though it does mean we will have to tweak osdep.h in future if a new vixl version should ever require any further similar #defines.) thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 9 February 2016 at 15:25, Peter Maydell <peter.maydell@linaro.org> wrote: >> Rearrange include directives so that we include osdep.h first. >> This has to be done manually because clean-includes doesn't >> handle C++. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> disas/arm-a64.cc | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/disas/arm-a64.cc b/disas/arm-a64.cc >> index d4d46d5..9280950 100644 >> --- a/disas/arm-a64.cc >> +++ b/disas/arm-a64.cc >> @@ -17,12 +17,13 @@ >> * along with this program. If not, see <http://www.gnu.org/licenses/>. >> */ >> >> -#include "vixl/a64/disasm-a64.h" >> - >> extern "C" { >> +#include "qemu/osdep.h" >> #include "disas/bfd.h" >> } >> >> +#include "vixl/a64/disasm-a64.h" >> + >> using namespace vixl; >> >> static Decoder *vixl_decoder = NULL; > > So this patch doesn't build on the old mingw32 compiler. I think this > is because this compiler is not C++11, and so its <stdint.h> doesn't > provide various macros for C++ unless __STDC_CONSTANT_MACROS, > __STDC_LIMIT_MACROS and __STDC_FORMAT_MACROS are defined before the > first inclusion of <stdint.h>. > > libvixl's globals.h defines these constants, but this only works if > globals.h is first-include, so making osdep.h first-include then > results in stdint.h being included before globals.h has a chance > to set the defines. > > What's the best way to deal with this? I can see a couple of options: > > (1) as a special case, for this file include disasm-a64.h before > osdep.h > (2) as a special case, for this file manually define the __STDC_* > before including osdep.h > (3) make osdep.h itself define the __STDC_* constants so it works > with C++-before-C++11 as well as with C and with C++11 (4) Tell old mingw32 to take a hike :) > I think I prefer (3) (though it does mean we will have to tweak > osdep.h in future if a new vixl version should ever require any > further similar #defines.) Doesn't sound like a deal-breaker to me.
On 18 February 2016 at 16:41, Markus Armbruster <armbru@redhat.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: >> What's the best way to deal with this? I can see a couple of options: >> >> (1) as a special case, for this file include disasm-a64.h before >> osdep.h >> (2) as a special case, for this file manually define the __STDC_* >> before including osdep.h >> (3) make osdep.h itself define the __STDC_* constants so it works >> with C++-before-C++11 as well as with C and with C++11 > > (4) Tell old mingw32 to take a hike :) I suspect it is not the only pre-C++11 compiler people in the wild will be using, though... thanks -- PMM
On 02/18/2016 08:49 AM, Peter Maydell wrote: > On 9 February 2016 at 15:25, Peter Maydell <peter.maydell@linaro.org> wrote: >> Rearrange include directives so that we include osdep.h first. >> This has to be done manually because clean-includes doesn't >> handle C++. >> >> -#include "vixl/a64/disasm-a64.h" >> - >> extern "C" { >> +#include "qemu/osdep.h" >> #include "disas/bfd.h" >> } >> >> +#include "vixl/a64/disasm-a64.h" >> + >> using namespace vixl; >> >> static Decoder *vixl_decoder = NULL; > > So this patch doesn't build on the old mingw32 compiler. I think this > is because this compiler is not C++11, and so its <stdint.h> doesn't > provide various macros for C++ unless __STDC_CONSTANT_MACROS, > __STDC_LIMIT_MACROS and __STDC_FORMAT_MACROS are defined before the > first inclusion of <stdint.h>. > (3) make osdep.h itself define the __STDC_* constants so it works > with C++-before-C++11 as well as with C and with C++11 > > I think I prefer (3) (though it does mean we will have to tweak > osdep.h in future if a new vixl version should ever require any > further similar #defines.) (3) has my vote as well; it's the approach used by gnulib (wherever practical, do whatever wrapping it takes to make all standard headers appear like the most modern version of said header, even on older platforms)
diff --git a/disas/arm-a64.cc b/disas/arm-a64.cc index d4d46d5..9280950 100644 --- a/disas/arm-a64.cc +++ b/disas/arm-a64.cc @@ -17,12 +17,13 @@ * along with this program. If not, see <http://www.gnu.org/licenses/>. */ -#include "vixl/a64/disasm-a64.h" - extern "C" { +#include "qemu/osdep.h" #include "disas/bfd.h" } +#include "vixl/a64/disasm-a64.h" + using namespace vixl; static Decoder *vixl_decoder = NULL;
Rearrange include directives so that we include osdep.h first. This has to be done manually because clean-includes doesn't handle C++. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- disas/arm-a64.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)