[Mesa-dev] [PATCH v3] swr: fix windows build break

[Ilia Mirkin]

On Wed, Jan 4, 2017 at 3:24 PM, Kyriazis, George
<george.kyriazis at intel.com> wrote:
>
>
>> -----Original Message-----
>> From: ibmirkin at gmail.com [mailto:ibmirkin at gmail.com] On Behalf Of Ilia
>> Mirkin
>> Sent: Wednesday, January 4, 2017 2:22 PM
>> To: Kyriazis, George <george.kyriazis at intel.com>
>> Cc: mesa-dev at lists.freedesktop.org
>> Subject: Re: [Mesa-dev] [PATCH v3] swr: fix windows build break
>>
>> On Wed, Jan 4, 2017 at 3:20 PM, George Kyriazis <george.kyriazis at intel.com>
>> wrote:
>> > wrap lp_bld_type.h around extern "C".
>> > Windows decorates global variables, so when used from .cpp files, need
>> > to use an undecorated version.
>> > ---
>> >  src/gallium/auxiliary/gallivm/lp_bld_type.h | 7 +++++++
>> >  src/gallium/drivers/swr/swr_screen.cpp      | 4 ----
>> >  2 files changed, 7 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_type.h
>> > b/src/gallium/auxiliary/gallivm/lp_bld_type.h
>> > index 7fb449f..afe8722 100644
>> > --- a/src/gallium/auxiliary/gallivm/lp_bld_type.h
>> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_type.h
>> > @@ -41,6 +41,10 @@
>> >  #include "pipe/p_compiler.h"
>> >  #include "gallivm/lp_bld.h"
>> >
>> > +#ifdef __cplusplus
>> > +extern "C" {
>> > +#endif
>> > +
>> >  /**
>> >   * Native SIMD architecture width available at runtime.
>> >   *
>> > @@ -449,5 +453,8 @@ lp_build_context_init(struct lp_build_context
>> > *bld,  unsigned  lp_build_count_ir_module(LLVMModuleRef module);
>> >
>> > +#ifdef __cplusplus
>> > +}
>> > +#endif
>> >
>> >  #endif /* !LP_BLD_TYPE_H */
>>
>> Changes to this file are
>>
>> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>
>> > diff --git a/src/gallium/drivers/swr/swr_screen.cpp
>> > b/src/gallium/drivers/swr/swr_screen.cpp
>> > index a9905d7..3b010fe 100644
>> > --- a/src/gallium/drivers/swr/swr_screen.cpp
>> > +++ b/src/gallium/drivers/swr/swr_screen.cpp
>> > @@ -39,10 +39,6 @@
>> >
>> >  #include "state_tracker/sw_winsys.h"
>> >
>> > -extern "C" {
>> > -#include "gallivm/lp_bld_limits.h"
>> > -}
>> > -
>>
>> I'm still confused by this hunk. It doesn't seem to be related to the change
>> description. It also doesn't define any function prototypes, so it doesn't really
>> need the extern "C" thing either.
>>
> This was part of the old code, hence the removal.  Yes, it doesn't quite fit with the rest; I can split to a different check-in.

Fine to keep it together (as far as I'm concerned). It just didn't
mesh with the change description. If you update the change description
to make a mention of this unnecessary include being removed, the whole
change gets my

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>

Cheers,

  -ilia