[cfe-commits] r62245 - in /cfe/trunk: lib/Sema/CMakeLists.txt lib/Sema/IdentifierResolver.h lib/Sema/Sema.h lib/Sema/SemaCXXScopeSpec.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaL

[Douglas Gregor]

On Jan 16, 2009, at 2:47 PM, Sebastian Redl wrote:

> Douglas Gregor wrote:
>> Can anyone point out a flaw in the code under discussion? It will
>> produce the appropriate warnings if we miss an enumerator, and it
>> doesn't seem to trigger any additional warnings under any particular
>> compiler or compilation flags. It's not broken, so why are we trying
>> to fix it?
>>
>
> The real question is, should we change other places to do it the same
> way? Should we adopt it as our new policy?
>
> Or does the existing policy mean that your code should change for the
> sake of uniformity?

I'm not convinced that we ever had a policy. At least, it's not in the  
LLVM coding standards, and IIRC it hasn't come up before.

It makes sense to

	default: assert(false && "Something");

when we don't expect that our switch statement will handle every case;  
this is most of what we see in Clang, because we're switching on the  
kind of a statement or declaration and we won't be handling everything.

In the code we're discussing, it does not make sense to add such a  
default case, because doing so suppresses a useful warning, while the  
code that's currently in Subversion does not appear to have any  
negative side effects.

If there is a policy on this, then either I'm missing something (some  
other compiler complains, perhaps) or the policy is wrong. At this  
point, I'm disinclined to make any change here unless someone could  
point me at either a written policy or show that the code we have  
causes some kind of problem on some compiler.

	- Doug