[PATCH] D62462: [llvm-objdump] Add warning messages if disassembly + source for problematic inputs

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:596
+    if(!WarnedNoDebugInfo) {
+      warn(Twine("failed to parse debug info which may be missing"));
+      WarnedNoDebugInfo = true;
----------------
mmpozulp wrote:
> grimar wrote:
> > You do not need `Twine(` here.
> It appears that I have created a problem by adding the new `warn(Twine)` function. If I delete `Twine(` here, I get
> 
> ```
> llvm-objdump.cpp:594:61: error: call of overloaded 'warn(const char [48])' is ambiguous
>        warn("failed to parse debug info which may be missing");
>                                                              ^
> llvm-objdump.cpp:375:6: note: candidate: 'void llvm::warn(llvm::StringRef)'
>  void warn(StringRef Message) {
>       ^~~~
> llvm-objdump.cpp:380:6: note: candidate: 'void llvm::warn(llvm::Twine)'
>  void warn(Twine Message) {
> 
> ```
> 
> I needed `warn(Twine)` for calls like `warn("a message " + VariableName)` and `warn(formatv("a message {0}", VariableName))` which create Twines.
> 
> Is there a way to change the `warn()` API to resolve the ambiguity? That way the compiler would not have to force users to pick between `warn(Twine)` and `warn(StringRef)` with a tedious `Twine()` or `StringRef()` constructor call.
I think we just need to change our `void warn(StringRef Message)` to `void warn(const Twine &Message)`.

(note: Twine.h says: "Twines should only be used accepted as const references in arguments, when an API wishes to accept possibly-concatenated strings.")


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62462