[PATCH] D21462: [PM] Make the the new pass manageg support fully generic extra arguments to run methods, both for transform passes and analysis passes.

[Chandler Carruth via llvm-commits]

chandlerc marked an inline comment as done.
chandlerc added a comment.

Thanks, submitting. Happy to follow-up on any of these issues (or other issues) in subsequent commits!


================
Comment at: include/llvm/IR/PassManager.h:975
@@ +974,3 @@
+template <typename AnalysisT, typename IRUnitT>
+struct RequireAnalysisPass<AnalysisT, IRUnitT, AnalysisManager<IRUnitT>>
+    : PassInfoMixin<
----------------
sanjoy wrote:
> Have you considered making this building block instead be:
> 
> ```
> template <typename AnalysisK, typename IRUnitT, typename... ExtraArgTs>
> struct RequireAnalysisPass<AnalysisK<IRUnitT, ExtraArgTs>, IRUnitT, AnalysisManager<IRUnitT, ExtraArgTs>> {
>   PreservedAnalyses run(IRUnitT &Arg, AnalysisManager<IRUnitT> &AM, ExtraArgTs... &&args) {
>     (void)AM.template getResult<AnalysisT>(Arg, std::forward(args));
> 
>     return PreservedAnalyses::all();
>   }
> };
> ```
> 
> and use cases that want something other than `std::forward` can specialize this differently?
I looked at this and it's kind of awkward to do....

If you do as you suggest, I'm worried using exactly one extra argument will become ambiguous with variants which specify a particular AnalysisManagerT and no extra arguments.

It essentially makes the specializations harder to control. And I have no use case today.

So for now, I think I prefer keeping this boring. If you see a reliable way to do what you suggest, please let me know and I'm happy to incorporate it, but so far I don't see one.

================
Comment at: unittests/IR/PassManagerTest.cpp:342
@@ +341,3 @@
+class CustomizedAnalysis : public AnalysisInfoMixin<CustomizedAnalysis> {
+public:
+  struct Result {
----------------
sanjoy wrote:
> Minor: just make this a `struct`?
I've been fairly consistently keeping the PassID static member private, which is why this is a class. I can reverse that if you feel strongly in a follow-up commit, but this isn't the only place where this happens.


https://reviews.llvm.org/D21462