[PATCH] Change final case of Driver::getToolChain to if/else/else/... NFC

Justin Bogner mail at justinbogner.com
Fri May 22 09:25:22 PDT 2015


Douglas Katzman <dougk at google.com> writes:
> Hi echristo,
>
> http://reviews.llvm.org/D9939
>
> Files:
>   lib/Driver/Driver.cpp
>
> Index: lib/Driver/Driver.cpp
> ===================================================================
> --- lib/Driver/Driver.cpp
> +++ lib/Driver/Driver.cpp
> @@ -2025,8 +2025,8 @@
>  
>  const ToolChain &Driver::getToolChain(const ArgList &Args,
>                                        StringRef DarwinArchName) const {
> -  llvm::Triple Target = computeTargetTriple(DefaultTargetTriple, Args,
> -                                            DarwinArchName);
> +  llvm::Triple Target =
> +      computeTargetTriple(DefaultTargetTriple, Args, DarwinArchName);
>  
>    ToolChain *&TC = ToolChains[Target.str()];
>    if (!TC) {
> @@ -2097,29 +2097,21 @@
>        }
>        break;
>      default:
> -      // TCE is an OSless target
>        if (Target.getArchName() == "tce") {
> +        // TCE is an OSless target
>          TC = new toolchains::TCEToolChain(*this, Target, Args);
> -        break;
> -      }
> -      // If Hexagon is configured as an OSless target
> -      if (Target.getArch() == llvm::Triple::hexagon) {
> +      } else if (Target.getArch() == llvm::Triple::hexagon) {
> +        // If Hexagon is configured as an OSless target

This comment (and the TCE one above) don't seem that useful. I'd
probably put a general comment after the default, like:

  default:
    // Handle OS-less targets

and then you can drop the {}s from the if/else chain since they're all
one-liners.

Otherwise, LGTM.

>          TC = new toolchains::Hexagon_TC(*this, Target, Args);
> -        break;
> -      }
> -      if (Target.getArch() == llvm::Triple::xcore) {
> +      } else if (Target.getArch() == llvm::Triple::xcore) {
>          TC = new toolchains::XCore(*this, Target, Args);
> -        break;
> -      }
> -      if (Target.isOSBinFormatELF()) {
> +      } else if (Target.isOSBinFormatELF()) {
>          TC = new toolchains::Generic_ELF(*this, Target, Args);
> -        break;
> -      }
> -      if (Target.isOSBinFormatMachO()) {
> +      } else if (Target.isOSBinFormatMachO()) {
>          TC = new toolchains::MachO(*this, Target, Args);
> -        break;
> +      } else {
> +        TC = new toolchains::Generic_GCC(*this, Target, Args);
>        }
> -      TC = new toolchains::Generic_GCC(*this, Target, Args);
>        break;
>      }
>    }
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list