[PATCH] D72802: [mlir] Introduce bare ptr calling convention for MemRefs in LLVM dialect

Diego Caballero via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 30 17:46:49 PST 2020


dcaballe marked an inline comment as done.
dcaballe added inline comments.


================
Comment at: mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h:154
 
-  // Extract an LLVM IR dialect type.
-  LLVM::LLVMType unwrap(Type type);
+  // Callbacks for customizing the type conversion.
+  LLVMTypeConverterCustomization customizations;
----------------
rriddle wrote:
> dcaballe wrote:
> > rriddle wrote:
> > > These should really be using ///
> > No strong opinion but just for my understanding... if I remember correctly, `///` is for Doxygen purposes so it's only needed for public interfaces and members (?). That's why `//` is used for private members and .cpp documentation. Isn't that the case? At least that's what I've seen around.
> > 
> > LLVM Coding Standards seem a bit ambiguous here:
> > https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
> > 
> > ```
> > Don’t duplicate the documentation comment in the header file and in the implementation file. Put the documentation comments for public APIs into the header file. Documentation comments for private APIs can go to the implementation file. In any case, implementation files can include additional comments (not necessarily in Doxygen markup) to explain implementation details as needed.
> > ```
> I prefer to keep comment style consistent to make the code base uniform, and less error-prone. Refactoring can often make things public that were private, and private that were public. By having a consistent style, there isn't a need to even think about any of that stuff. So for the sake of MLIR I've been trying to make sure that everyone is consistent.
Ok, that's reasonable. I'll change that before committing. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72802





More information about the llvm-commits mailing list