[PATCH] D78624: [CaptureTracking] Replace hardcoded constant to option. NFC.

Fedor Sergeev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 22 04:17:06 PDT 2020


fedor.sergeev added inline comments.


================
Comment at: llvm/include/llvm/Analysis/CaptureTracking.h:53-54
+                                  const DominatorTree *DT,
+                                  unsigned MaxUsesToExplore,
+                                  bool IncludeI = false);
+  bool PointerMayBeCapturedBefore(const Value *V, bool ReturnCaptures,
----------------
Flipping the order of two integer parameters is somewhat risky interface change, since it does not lead to errors or even warnings at old call-sites that pass values according to the old interface.



================
Comment at: llvm/include/llvm/Analysis/CaptureTracking.h:97
+
+  unsigned getDefaultMaxUsesToExploreForCaptureTracking();
 } // end namespace llvm
----------------
If you introduce this accessor function anyway, why dont you just specify it in all the default parameters.
Adding multiple overloads which just fill the default parameter looks too chatty.


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

https://reviews.llvm.org/D78624





More information about the llvm-commits mailing list