[PATCH] D143645: [llvm] Fix MCSubtargetInfo::checkFeatures to handle unknown features correctly

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/MC/MCSubtargetInfo.cpp:81-87
+    return true;
   } else {
     errs() << "'" << Feature << "' is not a recognized feature for this target"
            << " (ignoring feature)\n";
+    return false;
   }
+  return false;
----------------
avoid else-after-return (per LLVM coding conventions)

And it might even be worth inverting the previous condition (so it's `if (!FeatureEntry) { error stuff return false; } non-error stuff here`)

But also: we can't/don't usually print errors from arbitrary partts of LLVM - it's not use-LLVM-as-a-library friendly (the library client can't stop these messages from printing, render them in a different way/to a different mechanism (a log, a GUI, etc))

Are there other error handling conventions in use in nearby code you can find some inspiration from?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143645