Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'

[Chen Gang <gang.chen.5i5j@gmail.com>]

On 10/10/14 15:37, Peter Maydell wrote:
> On 10 October 2014 02:54, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>> I use the latest upstream gcc (which pulled from master in 2014-10-0?).
>> In my memory (not quite sure), the older version gcc may not notice
>> about this warning.
> 
> Hmm. I'll see if I can test with that gcc.
> 

It is not quite difficult to do that: get upstream source code, follow
the related document to build, then use it, it should be OK (I just do
like that).

>> But for me, the warning (compiler worries about) sounds reasonable, and
>> it's harmless to be fixed (after have a look, for me, they are declared,
>> but never be used).
> 
> It's a library. Other users of this code upstream will use these
> constants; it's just that we don't happen to.
> 
>>> The reason I'm reluctant to make changes to these files is
>>> that they're pulled in from a different upstream project
>>> (libvixl) so we should only fix critical problems in them,
>>> or it makes new versions harder to update to.
>>>
>>
>> Originally, I first try the Xilinx branch (Xilinx-master from Xilinx
>> github), yesterday, and found this issue, then I try upstream main
>> branch, found the same issue.
>>
>> For me, when add the related patch (which will use these variables in
>> 'libvixl'), then declare and set them in the related headers, again.
>> That will let other reviewers and readers easier understanding.
>>
>>  - removing them at present, is easy understanding.
>>
>>  - add them again when really need them, is also easy understanding.
> 
> But it's all changes which we would have to carry locally
> and then re-make every time we updated to a new libvixl.
> I definitely don't want to do that unless it's absolutely
> required.
> 

It is really a little complex, we almost can not touch this header file,
sorry for my original misunderstanding.

And I guess, "disas/arm-64.cc" is our own file (only for qemu, not from
libvixl upstream project). If really it is, we may do something in it to
avoid this warning, e.g.

  "#pragma GCC diagnostic ignored -Wunused-variable" (almost like "include/ui/qemu-pixman.h" have done).


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed