Re: [ros-dev] [ros-diffs] [gedmurphy] 52364: [BROWSEUI] - Set the prototypes to be long instead of LONG. This only worked in rbuild previously because WIDL incorrectly generates a LONG for long types. - define SHANDLE_PTR a...

[Colin Finck]

[e-mail] wrote:
> +// WIDL temp hack : [...]

Even though we all like to see a full MSVC build in the future, does 
this really justify such an even bigger hack on top of the existing 
ones? Would a proper fix really require that much more time?

If we don't want to end up in a new mess, this pretty much forces 
someone to fix it in the coming days or the hack/commit will be 
forgotten again. I'm certain about this, because nobody on #reactos-dev 
really knew the reason behind the older __ROS_LONG64__ hack either.

Alternatively, you could open a detailed bug report about this issue and 
assign some CMake people to it. But in any case, such hacks should be 
tracked in our Bugzilla, in particular when more information about a 
solution is known already.

- Colin

[Ged Murphy]

Colin Finck wrote:
> Even though we all like to see a full MSVC build in the future, does 
> this really justify such an even bigger hack on top of the existing 
> ones?

In what way is it an even bigger hack?
It merely changes the use of long back to how WIDL defines it in its
prototypes for the scope of the block.
> Would a proper fix really require that much more time?

It depends on what your idea of a proper fix is.
If you're referring to fixing it by removing __ROS_LONG64__, then that's not
a proper fix either.
WIDL needs to be fixed to stop outputting 'LONG' types when 'long' is used
in the interface file.

I don't have the time to fix widl, then any code which relied on the broken
output.

You're more than welcome to either revert it or fix it yourself.

Ged.

[Timo Kreuzer]

Am 19.06.2011 15:47, schrieb Ged Murphy:
> Colin Finck wrote:
> If you're referring to fixing it by removing __ROS_LONG64__, then 
> that's not
> a proper fix either.

I don't think __ROS_LONG64__ is a good workaround. It creates a lot of 
trouble. I had to disable it for amd64 completely since I would need to 
hack a lot of other code to make it work.
> WIDL needs to be fixed to stop outputting 'LONG' types when 'long' is used
> in the interface file.

Its done on purpose by wine. so we will have to do our own "hack" on it.

[Ged Murphy]

Timo Kreuzer wrote:
> Am 19.06.2011 15:47, schrieb Ged Murphy:
> > Colin Finck wrote:
> > If you're referring to fixing it by removing __ROS_LONG64__, then 
> > that's not
> > a proper fix either.
>
> I don't think __ROS_LONG64__ is a good workaround. It creates a lot of 
> trouble. I had to disable it for amd64 completely since I would need to 
> hack a lot of other code to make it work.

I agree that it should be removed.
Also removing it will possibly allow the removal of this WIDL hack in
browseui.
However it won't fix the fact that WIDL outputs LONG instead of long
> > WIDL needs to be fixed to stop outputting 'LONG' types when 'long' is

used
> > in the interface file.
>
> Its done on purpose by wine. so we will have to do our own "hack" on it.

If gcc doesn't complain about the LONG / long mismatch (msvc will), and as
long as we always plan to use MIDL for the msvc build, then maybe it's an
incorrect 'feature' we can live with.

[Timo Kreuzer]

Am 19.06.2011 15:27, schrieb Colin Finck:
> [e-mail] wrote:
> > +// WIDL temp hack : [...]
>
> Even though we all like to see a full MSVC build in the future, does 
> this really justify such an even bigger hack on top of the existing 
> ones? Would a proper fix really require that much more time?

I'm in the process of trying to remove the __ROS_LONG64__ hack. That 
should fix it, right?
I'll need to add a few -Wno-format, because sprintf for example is a 
function that gcc already knows about without the __attribute__((format 
(printf,x,y) )). So removing it doesn't help. Its a few modules only 
though. I think its worth it, because that __ROS_LONG64__ thing creates 
more problems then it solves.