[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |