oizma / angleproject

Automatically exported from code.google.com/p/angleproject
Other
0 stars 0 forks source link

unreachable default case in TIntermediate::addConversion #57

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. run Coverity on mozilla-central from something after 
http://hg.mozilla.org/mozilla-central/rev/582be9aca672

What is the expected output?
no warnings for unreachable code in TIntermediate::addConversion

What do you see instead?
warning in TIntermediate::addConversion for the "default" case in "switch 
(promoteTo)"

What version of the product are you using?
r367

On what operating system?
Linux with Coverity 4.5

Please provide any additional information below.
Static analysis can show that only certain values are legal past a certain 
point (in this case past the end of the "switch (op)" block. Personally I'd 
rather rely on tools that provide static analysis than rely on 
compilers/optimizers to detect such things.

I'd understand if you choose not to incorporate such a change. There is a 
school of "defensive programming" which suggests having such cases.

Original issue reported on code.google.com by timel...@gmail.com on 12 Oct 2010 at 4:20

Attachments:

GoogleCodeExporter commented 9 years ago
I would advise to keep the unreachable default case in place.

It may still be hit due to memory corruption, in which case it would be 
incredibly useful to be notified of it (especially when it's reproducible). 
Also, it only takes a minor code change to actually require this default case.

As far as I'm aware this also isn't performance critical code. But even so, 
optimized builds may still remove the unreachable code, but for debugging 
builds it is in my experience valuable to keep such code around.

Frankly, optimizing code is the task of optimizers and profilers, not code 
coverage tools. Unless this pops up as a hotspot, I would indeed suggest 
defensive programming.

Original comment by Nicolas....@gmail.com on 12 Oct 2010 at 10:37

GoogleCodeExporter commented 9 years ago
I agree with Nicolas here. Keeping the default case is completely harmless in 
this case.

Original comment by alokp@chromium.org on 14 Oct 2010 at 6:51