[PATCH] D30536: [tablegen][globalisel] Convert the SelectionDAG importer to a tree walking approach. NFC

[Ahmed Bougacha via Phabricator via llvm-commits]

ab added a comment.

This generally seems sensible, but if feasible it might be a good idea to replace the 'Expected<bool>' with 'Error'.



================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1281-1283
+  auto TrueOrError = importRulePredicates(M, P.getPredicates()->getValues());
+  if (auto Error = TrueOrError.takeError())
+    return std::move(Error);
----------------
Here and elsewhere: is it possible to fold the call into the if?  Something like:

  if (auto Err = importRulePredicates(M, P.getPredicates()->getValues()).takeError())
    return Err;

That also lets you avoid the not-very-descriptive 'TrueOrError'.

More generally, an 'Expected<bool>' that's always true seems odd.  If you don't have plans to add more information, maybe just return an Error?  Then the whole takeError vs weird-name problem goes away.


https://reviews.llvm.org/D30536