[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 0/6] Remove some usage of shadow variable



>>> On 27.10.15 at 18:41, <George.Dunlap@xxxxxxxxxxxxx> wrote:
> On Tue, Oct 27, 2015 at 4:03 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 27.10.15 at 16:39, <julien.grall@xxxxxxxxxx> wrote:
>>> I'd like to have some input to know whether turning on -Wshadow would be
>>> sensible in the future.
>>
>> I think there are cases where using a shadowed variable might make
>> sense, and hence I wouldn't want to see the warning turned on by
>> default.
> 
> Hmm, I'm having trouble coming up with good uses off the top of my
> head.  And are there any uses for which the value outweighs the value
> of having the warning?

First of all - macros using the ({}) gcc extension. And second variables
whose name is kind of natural (e.g. "d" for struct domain * instances)
but which are intentionally shadowing a larger scope one in order to
not clobber that one's value.

> And in line with my response to Andrew -- could we enable -Wshadow
> until we find a use for shadowing whose value outweighs the risks of
> building without it?

Risking - along the lines of what Andrew said - build breakage for
random people, just due to the gcc version they happen to use?
I'm usually getting pretty upset when running into problems specific
to certain gcc versions, where people fairly clearly didn't think about
making their code sufficiently general. I don't know how people will
feel if we intentionally break their build (well, not really intentionally,
but we'd intentionally take the risk of doing so).

And then, simply based on the patches that Julien sent so far: Are
we suspecting any bugs because of shadowing variables? None of
his patches fixed anything; they were just cleanup for cases where
the shadowing was pointless (and perhaps not even intended).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.