[cfe-commits] r173171 - in /cfe/trunk/lib/Serialization: ASTCommon.cpp ASTCommon.h ASTReader.cpp

[John McCall]

On Jan 22, 2013, at 4:00 PM, Douglas Gregor <dgregor at apple.com> wrote:
> On Jan 22, 2013, at 3:58 PM, John McCall <rjmccall at apple.com> wrote:
>> On Jan 22, 2013, at 9:08 AM, Douglas Gregor <dgregor at apple.com> wrote:
>>> Author: dgregor
>>> Date: Tue Jan 22 11:08:30 2013
>>> New Revision: 173171
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=173171&view=rev
>>> Log:
>>> Make getDefinitiveDeclContext() actually return a DeclContext, as one
>>> would expect, and clean up the return/break inconsistencies. Thanks,
>>> Sebastian!
>>> 
>>> Modified:
>>>  cfe/trunk/lib/Serialization/ASTCommon.cpp
>>>  cfe/trunk/lib/Serialization/ASTCommon.h
>>>  cfe/trunk/lib/Serialization/ASTReader.cpp
>>> 
>>> Modified: cfe/trunk/lib/Serialization/ASTCommon.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTCommon.cpp?rev=173171&r1=173170&r2=173171&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Serialization/ASTCommon.cpp (original)
>>> +++ cfe/trunk/lib/Serialization/ASTCommon.cpp Tue Jan 22 11:08:30 2013
>>> @@ -87,7 +87,8 @@
>>> return R;
>>> }
>>> 
>>> -const Decl *serialization::getDefinitiveDeclContext(const DeclContext *DC) {
>>> +const DeclContext *
>>> +serialization::getDefinitiveDeclContext(const DeclContext *DC) {
>>> switch (DC->getDeclKind()) {
>>> // These entities may have multiple definitions.
>>> case Decl::TranslationUnit:
>>> @@ -100,7 +101,7 @@
>>> case Decl::Record:
>>>   if (const TagDecl *Def = cast<TagDecl>(DC)->getDefinition())
>>>     return Def;
>>> -    break;
>>> +    return 0;
>>> 
>>> // FIXME: These can be defined in one place... except special member
>>> // functions and out-of-line definitions.
>>> @@ -122,25 +123,25 @@
>>> case Decl::ObjCCategory:
>>> case Decl::ObjCCategoryImpl:
>>> case Decl::ObjCImplementation:
>>> -    return cast<Decl>(DC);
>>> +    return DC;
>>> 
>>> case Decl::ObjCProtocol:
>>>   if (const ObjCProtocolDecl *Def
>>>         = cast<ObjCProtocolDecl>(DC)->getDefinition())
>>>     return Def;
>>> -    break;
>>> +    return 0;
>>> 
>>> // FIXME: These are defined in one place, but properties in class extensions
>>> // end up being back-patched into the main interface. See
>>> // Sema::HandlePropertyInClassExtension for the offending code.
>>> case Decl::ObjCInterface:
>>> -    break;
>>> +    return 0;
>>> 
>>> default:
>>>   llvm_unreachable("Unhandled DeclContext in AST reader");
>>> }
>>> 
>>> -  return 0;
>>> +  llvm_unreachable("Unhandled decl kind");
>>> }
>> 
>> Why do you have both an unreachable default and an unreachable fall-through?
> 
> Couldn't remember if older GCCs were smart enough to suppress -Wswitch errors without the llvm_unreachable at the bottom. I'll remove it, and we'll see.

I guess my real question is why you need a default case.  We should be able to macro-metaprogram this.

John.