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

Reply via email to