dberris added inline comments.
================ Comment at: lib/Driver/Tools.cpp:4903-4906 if (Triple.getOS() == llvm::Triple::Linux && (Triple.getArch() == llvm::Triple::arm || - Triple.getArch() == llvm::Triple::x86_64)) { + Triple.getArch() == llvm::Triple::x86_64 || + Triple.getArch() == llvm::Triple::aarch64)) { ---------------- rSerge wrote: > dberris wrote: > > I'm wondering whether it's worth turning this into a `switch` statement now > > that we have more than two supported architectures? > I think that would lead to more awkward code: there wouldn't be a single > decision outcome point (like the current `else` block), so to avoid > duplicating the code which currently prints the message, a `bool` variable > would be needed. I think it's more neat to just enumerate all the OS&CPU > combinations in the `if` condition for now. > This place is not performance-critical and the compiler should convert > appropriate `if`s into `switch`es anyway. This is an issue of making it more readable, something like: ``` if (Triple.getOS() != llvm::Triple::Linux) D.Diag(...) << ...; // Unsupported OS. switch (Triple.getArch()) { case llvm::Triple::arm: case llvm::Triple::x86_64: case llvm::Tripe::aarch64: // Supported. break; default: D.Diag(...) << ...; } ``` This way any new architectures become supported, they just get added to the list of cases that short-circuit. https://reviews.llvm.org/D26415 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits