On Mon, Oct 13, 2014 at 2:53 PM, Felipe Franciosi [off-list ref] wrote:
On Mon, Oct 13, 2014 at 9:12 PM, Junio C Hamano [off-list ref] wrote:
quoted
FNV/I/IDIV10/0 covers all the possibilities of (method & 3), I would
have to say that the compiler needs to be fixed.
Or insert "default:" just before "case HASH_METHOD_0:" line?
I dunno.
Hmm... The "default:" would work, but is it really that bad to initialise a
local variable in this case?
In any case, the compilation warning is annoying. Do you prefer the default
or the initialisation?
If I really had to choose between the two, adding a useless initialization
would be the less harmful choice. Adding a meaningless "default:" robs
another chance from the compilers to diagnose a future breakage we
might add (namely, we may extend methods and forget to write a
corresponding case arm for the new method value, which a smart
compiler can and do diagnose as a switch that does not handle
all the possible values.
Thanks.
On Tue, Oct 14, 2014 at 2:13 AM, Junio C Hamano [off-list ref] wrote:
On Mon, Oct 13, 2014 at 2:53 PM, Felipe Franciosi [off-list ref] wrote:
quoted
On Mon, Oct 13, 2014 at 9:12 PM, Junio C Hamano [off-list ref] wrote:
quoted
FNV/I/IDIV10/0 covers all the possibilities of (method & 3), I would
have to say that the compiler needs to be fixed.
Or insert "default:" just before "case HASH_METHOD_0:" line?
I dunno.
Hmm... The "default:" would work, but is it really that bad to initialise a
local variable in this case?
In any case, the compilation warning is annoying. Do you prefer the default
or the initialisation?
If I really had to choose between the two, adding a useless initialization
would be the less harmful choice. Adding a meaningless "default:" robs
another chance from the compilers to diagnose a future breakage we
might add (namely, we may extend methods and forget to write a
corresponding case arm for the new method value, which a smart
compiler can and do diagnose as a switch that does not handle
all the possible values.
Thanks.
I see your point; the code is correct today because it covers all
cases. Nevertheless, some versions of gcc (the one I used was 4.1.2
from CentOS 5.10 -- haven't tested others) might generate an annoying
warning.
Noting that, I also like my code to compile as cleanly as possible in
all environments that it might be used. Being a bit defensive in that
sense and initialising local variables is what I would do. On top of
that (and putting the compiler flaw aside for a moment), having it
sensibly initialised is another way of protecting the code against
errors introduced in the future.
What do you think?
Cheers,
F.
On Tue, Oct 14, 2014 at 4:44 AM, Felipe Franciosi [off-list ref] wrote:
On Tue, Oct 14, 2014 at 2:13 AM, Junio C Hamano [off-list ref] wrote:
quoted
If I really had to choose between the two, adding a useless initialization
would be the less harmful choice. Adding a meaningless "default:" robs
...
Being a bit defensive in that
sense and initialising local variables is what I would do. On top of
that (and putting the compiler flaw aside for a moment), having it
sensibly initialised is another way of protecting the code against
errors introduced in the future.
That is a false sense of safety. You will not know if the new method
introduced in the future would behave sensibly if the variable is left
in a state the blanket initialization created, so setting it to 0 upfront
is not really being defensive; you would rob compilers a chance
to notice something is amiss in the future code with the initialization,
just like a "default:" would. We need to accept that both are not about
being defensive but are ways to work around stupid compilers from
reporting false positives.
I am not saying that we should not do a work around. I am only
saying that it is wrong to try selling such a work around as a defensive
good practice, which is not.
What do you think?
Again, if I really had to choose between the two, adding a useless
initialization would be the less harmful choice, as the other one has
an extra downside.