[PATCH] D23701: [PM] Rework the new PM support for building the ModuleSummaryIndex to directly produce the index as the value type result.

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.

================
Comment at: include/llvm/Analysis/ModuleSummaryAnalysis.h:45
@@ -78,3 +44,3 @@
 
-  const ModuleSummaryIndex &run(Module &M, ModuleAnalysisManager &AM);
+  ModuleSummaryIndex run(Module &M, ModuleAnalysisManager &AM);
 };
----------------
chandlerc wrote:
> mehdi_amini wrote:
> > You define a typedef for `Result` two lines above, why not using it here? 
> > 
> > I don't think it is used anywhere, if it is only used by the pass manager infrastructure, maybe add a comment above the typedef.
> Happy to use the typedef if you prefer.
> 
> But the typedef is for the pass manager infrastructure. I can comment it, but it is a fairly pervasive pattern so not sure how much value that comment adds. Thoughts? Would you like a comment on all the Result typedefs?
Well you're right, since it is new we're not used to see it everywhere, but since it is gonna be everywhere we probably won't need to replicate the one line comment on each and every instance.


https://reviews.llvm.org/D23701