On Sat, Jan 21, 2012 at 5:11 AM, Sebastian Redl <[email protected]> wrote: > > On 20.01.2012, at 22:50, David Blaikie wrote: > > Author: dblaikie > Date: Fri Jan 20 15:50:17 2012 > New Revision: 148577 > > URL: http://llvm.org/viewvc/llvm-project?rev=148577&view=rev > Log: > More dead code removal (using -Wunreachable-code) > > > Modified: cfe/trunk/lib/AST/Decl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=148577&r1=148576&r2=148577&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/Decl.cpp (original) > +++ cfe/trunk/lib/AST/Decl.cpp Fri Jan 20 15:50:17 2012 > @@ -48,8 +48,6 @@ > case VisibilityAttr::Protected: > return ProtectedVisibility; > } > - > - return DefaultVisibility; > } > > > Should an llvm_unreachable should be added here?
Clang knows it's unreachable because the switch is fully covered - GCC doesn't, but won't complain because the function still eventually returns. I only added llvm_unreachables back in where necessary to silence GCC warnings, though it's possible to add them more liberally for code documentation, etc. I don't mind either way. > Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=148577&r1=148576&r2=148577&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Fri Jan 20 15:50:17 2012 > @@ -2175,36 +2170,28 @@ > case BuiltinType::UChar: > return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequb_p : > llvm::Intrinsic::ppc_altivec_vcmpgtub_p; > - break; > case BuiltinType::Char_S: > case BuiltinType::SChar: > return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequb_p : > llvm::Intrinsic::ppc_altivec_vcmpgtsb_p; > - break; > case BuiltinType::UShort: > return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequh_p : > llvm::Intrinsic::ppc_altivec_vcmpgtuh_p; > - break; > case BuiltinType::Short: > return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequh_p : > llvm::Intrinsic::ppc_altivec_vcmpgtsh_p; > - break; > case BuiltinType::UInt: > case BuiltinType::ULong: > return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequw_p : > llvm::Intrinsic::ppc_altivec_vcmpgtuw_p; > - break; > case BuiltinType::Int: > case BuiltinType::Long: > return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequw_p : > llvm::Intrinsic::ppc_altivec_vcmpgtsw_p; > - break; > case BuiltinType::Float: > return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpeqfp_p : > llvm::Intrinsic::ppc_altivec_vcmpgtfp_p; > - break; > } > - return llvm::Intrinsic::not_intrinsic; > } > > > And here. In this case both GCC and Clang know that the end of the function is unreachable (& don't give -Wreturn-type warnings) because the switch is fully covered (all cases /and/ a default have return statements). Purely for documentation purposes, one could be added, but it would pretty much be a compiler bug if they were hit (unlike the previous case where the unreachable could be hit by relatively easy subversion of the type system (wedging a non-enum value into the enum)) - David _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
