You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openoffice.apache.org by Don Lewis <tr...@apache.org> on 2019/11/05 18:54:17 UTC
what is this supposed to do?
I don't understand what this strange little bit of code (line 611 of
basebmp/inc/basebmp/packedpixeliterator.hxx) is supposed to do:
value_type get(difference_type const & d) const
{
const int remainder( x(d.x) % num_intraword_positions );
^^^^^^
return (unsigned_cast<value_type>(*current(d.x,d.y) &
get_mask<value_type, bits_per_pixel, MsbFirst>(remainder))
>> get_shift<num_intraword_positions, bits_per_pixel, MsbFirst>(remainder));
}
I've never seen any compiler complaints about it before, but gcc 9
throws an error:
../inc/basebmp/packedpixeliterator.hxx: In member function 'basebmp::PackedPixel
Iterator<Valuetype, bits_per_pixel, MsbFirst>::value_type basebmp::PackedPixelIt
erator<Valuetype, bits_per_pixel, MsbFirst>::get(const difference_type&) const':
../inc/basebmp/packedpixeliterator.hxx:611:35: error: expression cannot be used
as a function
611 | const int remainder( x(d.x) % num_intraword_positions );
| ^
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org
Re: what is this supposed to do?
Posted by Branko Čibej <br...@apache.org>.
On 05.11.2019 19:54, Don Lewis wrote:
> I don't understand what this strange little bit of code (line 611 of
> basebmp/inc/basebmp/packedpixeliterator.hxx) is supposed to do:
>
> value_type get(difference_type const & d) const
> {
> const int remainder( x(d.x) % num_intraword_positions );
> ^^^^^^
> return (unsigned_cast<value_type>(*current(d.x,d.y) &
> get_mask<value_type, bits_per_pixel, MsbFirst>(remainder))
> >> get_shift<num_intraword_positions, bits_per_pixel, MsbFirst>(remainder));
> }
>
> I've never seen any compiler complaints about it before, but gcc 9
> throws an error:
>
> ../inc/basebmp/packedpixeliterator.hxx: In member function 'basebmp::PackedPixel
> Iterator<Valuetype, bits_per_pixel, MsbFirst>::value_type basebmp::PackedPixelIt
> erator<Valuetype, bits_per_pixel, MsbFirst>::get(const difference_type&) const':
> ../inc/basebmp/packedpixeliterator.hxx:611:35: error: expression cannot be used
> as a function
> 611 | const int remainder( x(d.x) % num_intraword_positions );
> | ^
Gcc 9 is right, 'x' is a MoveX which is an integer as far as I can see.
I have no idea what this means either, or how it can be valid code.
Unless there's some really heavy macro magic going on behind the scenes,
but that doesn't make much sense given the rest of the code in that class.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org
Re: what is this supposed to do?
Posted by Branko Čibej <br...@apache.org>.
On 06.11.2019 18:06, Don Lewis wrote:
> On 6 Nov, Branko Čibej wrote:
>> On 05.11.2019 20:20, Peter Kovacs wrote:
>>> I am not sure why, but if you use gcc++98 std it compiles. Maybe it
>>> optimizes the code since it might be dead code.
>> That would be a very, very strange way to go about things. To figure out
>> something is dead code, you have to at *least* parse it first, and the
>> syntax error is pretty glaring.
(Oops ... it's not, strictly speaking, a syntax error at all.)
>> It's not impossible that the function is
>> never called and hence previous (IMO, buggy) versions of GCC never got
>> to generate the template function.
> It's not just gcc, clang also is happy, at least in gnu++98 mode. I do
> remember seeing this problem before, which was probably after clang
> switched its default to a newer C++ standard and before I switched the
> FreeBSD port to always build in gnu++98 mode.
>
> My suspicion is that the source is getting tokenized, but since the
> method is defined in a header and will get inlined where it gets used,
> it is being treated a lot like the body of a macro and the deeper
> analysis of the code is getting deferred until the method gets called
> somewhere.
That makes sense, yes. Oooh, I could've got away with so many nasty
delayed-action mines in the code if I'd only known some compilers
behaved in this "interesting" way. :D
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org
Re: what is this supposed to do?
Posted by Don Lewis <tr...@apache.org>.
On 6 Nov, Branko Čibej wrote:
> On 05.11.2019 20:20, Peter Kovacs wrote:
>> I am not sure why, but if you use gcc++98 std it compiles. Maybe it
>> optimizes the code since it might be dead code.
>
> That would be a very, very strange way to go about things. To figure out
> something is dead code, you have to at *least* parse it first, and the
> syntax error is pretty glaring. It's not impossible that the function is
> never called and hence previous (IMO, buggy) versions of GCC never got
> to generate the template function.
It's not just gcc, clang also is happy, at least in gnu++98 mode. I do
remember seeing this problem before, which was probably after clang
switched its default to a newer C++ standard and before I switched the
FreeBSD port to always build in gnu++98 mode.
My suspicion is that the source is getting tokenized, but since the
method is defined in a header and will get inlined where it gets used,
it is being treated a lot like the body of a macro and the deeper
analysis of the code is getting deferred until the method gets called
somewhere.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org
Re: what is this supposed to do?
Posted by Branko Čibej <br...@apache.org>.
On 05.11.2019 20:20, Peter Kovacs wrote:
> I am not sure why, but if you use gcc++98 std it compiles. Maybe it
> optimizes the code since it might be dead code.
That would be a very, very strange way to go about things. To figure out
something is dead code, you have to at *least* parse it first, and the
syntax error is pretty glaring. It's not impossible that the function is
never called and hence previous (IMO, buggy) versions of GCC never got
to generate the template function.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org
Re: what is this supposed to do?
Posted by Peter Kovacs <pe...@apache.org>.
If it is not used we should drop the code. I do not believe anyone will use this part.
The whole file is only used to calculate Grescale palette and some other predifened bitmap. The object created I think behind all this is bitmap device (or similar) which I think should be replaced at some point.
Am 6. November 2019 00:48:34 MEZ schrieb Don Lewis <tr...@apache.org>:
>On 5 Nov, Peter Kovacs wrote:
>> I am not sure why, but if you use gcc++98 std it compiles. Maybe it
>> optimizes the code since it might be dead code.
>
>It breaks the build in any mode for me with gcc9. I vaguely remember
>running across this code before, maybe with clang after it's default
>language version changed and before I changed the FreeBSD port to
>gnu++89.
>
>It does seem to be unused. I can comment out the entire method and the
>build still succeeds.
>
>I'll probably commit that along with a note. If someone needs this
>method in the future, they can presumably figure out what the correct
>fix is.
>
>> I rewrote the whole code, and used one class instead of 4 macros that
>> copy paste from each other. I can post the code.
>>
>> But I do not know how to test or apply it into the rest. :P
>>
>>
>> On 05.11.19 19:54, Don Lewis wrote:
>>> I don't understand what this strange little bit of code (line 611 of
>>> basebmp/inc/basebmp/packedpixeliterator.hxx) is supposed to do:
>>>
>>> value_type get(difference_type const & d) const
>>> {
>>> const int remainder( x(d.x) % num_intraword_positions );
>>> ^^^^^^
>>> return (unsigned_cast<value_type>(*current(d.x,d.y) &
>>> get_mask<value_type,
>bits_per_pixel, MsbFirst>(remainder))
>>> >> get_shift<num_intraword_positions,
>bits_per_pixel, MsbFirst>(remainder));
>>> }
>>>
>>> I've never seen any compiler complaints about it before, but gcc 9
>>> throws an error:
>>>
>>> ../inc/basebmp/packedpixeliterator.hxx: In member function
>'basebmp::PackedPixel
>>> Iterator<Valuetype, bits_per_pixel, MsbFirst>::value_type
>basebmp::PackedPixelIt
>>> erator<Valuetype, bits_per_pixel, MsbFirst>::get(const
>difference_type&) const':
>>> ../inc/basebmp/packedpixeliterator.hxx:611:35: error: expression
>cannot be used
>>> as a function
>>> 611 | const int remainder( x(d.x) %
>num_intraword_positions );
>>> | ^
>>>
>>>
>>>
>---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
>>> For additional commands, e-mail: dev-help@openoffice.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
>> For additional commands, e-mail: dev-help@openoffice.apache.org
>>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
>For additional commands, e-mail: dev-help@openoffice.apache.org
Re: what is this supposed to do?
Posted by Don Lewis <tr...@apache.org>.
On 5 Nov, Peter Kovacs wrote:
> I am not sure why, but if you use gcc++98 std it compiles. Maybe it
> optimizes the code since it might be dead code.
It breaks the build in any mode for me with gcc9. I vaguely remember
running across this code before, maybe with clang after it's default
language version changed and before I changed the FreeBSD port to
gnu++89.
It does seem to be unused. I can comment out the entire method and the
build still succeeds.
I'll probably commit that along with a note. If someone needs this
method in the future, they can presumably figure out what the correct
fix is.
> I rewrote the whole code, and used one class instead of 4 macros that
> copy paste from each other. I can post the code.
>
> But I do not know how to test or apply it into the rest. :P
>
>
> On 05.11.19 19:54, Don Lewis wrote:
>> I don't understand what this strange little bit of code (line 611 of
>> basebmp/inc/basebmp/packedpixeliterator.hxx) is supposed to do:
>>
>> value_type get(difference_type const & d) const
>> {
>> const int remainder( x(d.x) % num_intraword_positions );
>> ^^^^^^
>> return (unsigned_cast<value_type>(*current(d.x,d.y) &
>> get_mask<value_type, bits_per_pixel, MsbFirst>(remainder))
>> >> get_shift<num_intraword_positions, bits_per_pixel, MsbFirst>(remainder));
>> }
>>
>> I've never seen any compiler complaints about it before, but gcc 9
>> throws an error:
>>
>> ../inc/basebmp/packedpixeliterator.hxx: In member function 'basebmp::PackedPixel
>> Iterator<Valuetype, bits_per_pixel, MsbFirst>::value_type basebmp::PackedPixelIt
>> erator<Valuetype, bits_per_pixel, MsbFirst>::get(const difference_type&) const':
>> ../inc/basebmp/packedpixeliterator.hxx:611:35: error: expression cannot be used
>> as a function
>> 611 | const int remainder( x(d.x) % num_intraword_positions );
>> | ^
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
>> For additional commands, e-mail: dev-help@openoffice.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
> For additional commands, e-mail: dev-help@openoffice.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org
Re: what is this supposed to do?
Posted by Peter Kovacs <Pe...@Apache.org>.
I am not sure why, but if you use gcc++98 std it compiles. Maybe it
optimizes the code since it might be dead code.
I rewrote the whole code, and used one class instead of 4 macros that
copy paste from each other. I can post the code.
But I do not know how to test or apply it into the rest. :P
On 05.11.19 19:54, Don Lewis wrote:
> I don't understand what this strange little bit of code (line 611 of
> basebmp/inc/basebmp/packedpixeliterator.hxx) is supposed to do:
>
> value_type get(difference_type const & d) const
> {
> const int remainder( x(d.x) % num_intraword_positions );
> ^^^^^^
> return (unsigned_cast<value_type>(*current(d.x,d.y) &
> get_mask<value_type, bits_per_pixel, MsbFirst>(remainder))
> >> get_shift<num_intraword_positions, bits_per_pixel, MsbFirst>(remainder));
> }
>
> I've never seen any compiler complaints about it before, but gcc 9
> throws an error:
>
> ../inc/basebmp/packedpixeliterator.hxx: In member function 'basebmp::PackedPixel
> Iterator<Valuetype, bits_per_pixel, MsbFirst>::value_type basebmp::PackedPixelIt
> erator<Valuetype, bits_per_pixel, MsbFirst>::get(const difference_type&) const':
> ../inc/basebmp/packedpixeliterator.hxx:611:35: error: expression cannot be used
> as a function
> 611 | const int remainder( x(d.x) % num_intraword_positions );
> | ^
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
> For additional commands, e-mail: dev-help@openoffice.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org