[PATCH] D81236: Improve LegacyPassManager API to correctly report modified status

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

//If// we're happy that `getAnalysis` can cause changes, //and// we don't want to update all callers of `getAnalysis` to expect a `std::pair<AnalysisType &, bool>` result, then your changes look good to me.



================
Comment at: llvm/include/llvm/PassAnalysisSupport.h:265
   // vector.
-  Pass *ResultPass = Resolver->findImplPass(this, PI, F);
+  auto PassStatus = Resolver->findImplPass(this, PI, F);
+
----------------
How about `std::tie(Pass, Changed) = findImplPass(...)` to avoid the use of std::get ?


================
Comment at: llvm/include/llvm/PassAnalysisSupport.h:271
+    *Changed |= std::get<bool>(PassStatus);
+  assert((Changed || !std::get<bool>(PassStatus)) &&
+         "A pass trigged a code update but the update status is lost");
----------------
I'd find `else assert(!std::get<bool>(PassStatus) && "...")` easier to understand.


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

https://reviews.llvm.org/D81236