[PATCH] D113082: [lld-macho] Implement -arch_errors_fatal

Keith Smiley via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 3 21:35:16 PDT 2021


keith added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:793
+    if (config->errorForArchMismatch)
+      error(toString(this) + " has architecture " + getArchitectureName(arch) +
+            " which is incompatible with target architecture " +
----------------
int3 wrote:
> keith wrote:
> > int3 wrote:
> > > could we factor out the arguments somehow? E.g. something like `auto f = config->errorForArchMismatch ? error : warning`? (not sure if that compiles though)
> > I couldn't get a version of assigning the functions to a variable to work due to `reference to overloaded function could not be resolved`, I might have been doing something wrong?
> > 
> > Either way for now I added a simple lambda to de-duplicate the string, lmk what you think.
> > 
> > Note the whole reason I didn't assign the string to a variable instead was it seems like Twine is prime to use-after-free issues so the string was garbled in the output, clang-tidy also warned about that.
> ah yeah I guess we need to use static_cast to indicate which overload we want. this should work:
> 
>      auto msg = config->errorForArchMismatch
>                     ? static_cast<void (*)(const Twine &)>(error)
>                     : warn;
>      msg(toString(this) + " has architecture " + getArchitectureName(arch) +
>          " which is incompatible with target architecture " +
>          getArchitectureName(config->arch()));
Ah thanks, I tried wrapping the whole ternary in the static_cast, but this works


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113082



More information about the llvm-commits mailing list