[cfe-commits] r140068 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/Specifiers.h lib/AST/Decl.cpp lib/AST/DeclPrinter.cpp lib/CodeGen/CGDecl.cpp lib/CodeGen/CGOpenCLRuntime.cpp lib/CodeGen/CGOpenCLRuntime.h lib/CodeGen/CMakeLists.txt lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/Sema/SemaDecl.cpp test/CodeGenOpenCL/local.cl test/SemaOpenCL/local.cl

[Peter Collingbourne]

On Mon, Sep 19, 2011 at 02:27:49PM -0700, John McCall wrote:
> This looks good;  very minor comments below:
> 
> On Sep 19, 2011, at 2:14 PM, Peter Collingbourne wrote:
> > Modified: cfe/trunk/lib/AST/Decl.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=140068&r1=140067&r2=140068&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/AST/Decl.cpp (original)
> > +++ cfe/trunk/lib/AST/Decl.cpp Mon Sep 19 16:14:35 2011
> > @@ -1119,12 +1119,13 @@
> > 
> > const char *VarDecl::getStorageClassSpecifierString(StorageClass SC) {
> >   switch (SC) {
> > -  case SC_None:          break;
> > -  case SC_Auto:          return "auto"; break;
> > -  case SC_Extern:        return "extern"; break;
> > -  case SC_PrivateExtern: return "__private_extern__"; break;
> > -  case SC_Register:      return "register"; break;
> > -  case SC_Static:        return "static"; break;
> > +  case SC_None:                 break;
> > +  case SC_Auto:                 return "auto"; break;
> > +  case SC_Extern:               return "extern"; break;
> > +  case SC_OpenCLWorkGroupLocal: return "<<work-group-local>>"; break;
> > +  case SC_PrivateExtern:        return "__private_extern__"; break;
> > +  case SC_Register:             return "register"; break;
> > +  case SC_Static:               return "static"; break;
> >   }
> 
> I know you aren't responsible for this, but would you mind killing
> the unnecessary 'break's on these?

Done, r140142.  I'm not entirely sure why I didn't remove them myself.

> 
> > ==============================================================================
> > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Sep 19 16:14:35 2011
> > @@ -6131,6 +6147,9 @@
> >   case SC_Register:
> >     Error = 4;
> >     break;
> > +  case SC_OpenCLWorkGroupLocal:
> > +    assert(0 && "Unexpected storage class");
> > +    break;
> 
> We generally prefer llvm_unreachable to assert(0).

Changed.

Thanks,
-- 
Peter