[cfe-commits] r148577 - in /cfe/trunk: lib/AST/ lib/Analysis/ lib/CodeGen/ lib/Frontend/ lib/Index/ lib/Parse/ lib/Sema/ lib/StaticAnalyzer/Checkers/ lib/StaticAnalyzer/Core/ tools/libclang/

[David Blaikie]

On Sat, Jan 21, 2012 at 5:11 AM, Sebastian Redl
<sebastian.redl at getdesigned.at> 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