[PATCH] D27502: [PM] Edit comments on PM Proxy and utility classes.

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 4 20:30:34 PST 2017


mehdi_amini added inline comments.


================
Comment at: llvm/include/llvm/IR/PassManager.h:882-883
 
-/// \brief A module analysis which acts as a proxy for a function analysis
-/// manager.
+/// \brief An analysis over a "larger" IR unit that provides access to an
+/// analysis manager over a "smaller" IR unit.
 ///
----------------
chandlerc wrote:
> FWIW, I continue to prefer "outer" and "inner". There is an inherent requirement that the smaller IR units in question be *contained* by the larger units. I'd like words that help indicate that relationship which is a stronger statement than their size IMO.
+1 with this and all the other naming remarks below.


================
Comment at: llvm/include/llvm/IR/PassManager.h:1239
 
-}
+} // namespace llvm
 
----------------
chandlerc wrote:
> This isn't common for full-file-scoped namespaces in LLVM. See the last paragraph of http://llvm.org/docs/CodingStandards.html#namespace-indentation
I believe it is quite common, in llvm/include there are 941 "namespace llvm" and 478 have a closing comment.


https://reviews.llvm.org/D27502





More information about the llvm-commits mailing list