[PATCH] D87697: Do not construct std::string from nullptr

[Marshall Clow via Phabricator via llvm-commits]

mclow.lists added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp:1277
     llvm_unreachable("unexpected type");
     break;
   case Type::IntegerTyID: {
----------------
nikic wrote:
> @mclow.lists There already is a call to llvm_unreachable here ^ so the second call to unreachable seems redundant.
I know there are compilers that will warn if you get to the end of a fn and there's no return statement there.  Don't know how good clang is about analyzing all the paths through a function and determining that all of them lead to a return (and/or unreachable)

To make it easy on the compiler (and the reader), if I was writing this, I would move the default case to the end, and make the function look end look like this:

```
    else
      return "u32";
  default:
      break;
  }
  llvm_unreachable("unexpected type");
  return "";
}
```



================
Comment at: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp:1299
   case Type::PointerTyID:
     if (static_cast<const NVPTXTargetMachine &>(TM).is64Bit())
       if (useB4PTR)
----------------
But I would also rewrite this chunk as:
```
if (static_cast<const NVPTXTargetMachine &>(TM).is64Bit())
    return useB4PTR ? "b64" : "u64";
else
    return useB4PTR ? "b32" : "u32";
```

But now I'm indulging in feature-creep :-) so you can ignore this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87697/new/

https://reviews.llvm.org/D87697