[PATCH] D67646: [llvm][TextAPI] adding inlining reexported libraries support

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 2 00:30:13 PDT 2020


jhenderson added inline comments.


================
Comment at: llvm/include/llvm/TextAPI/MachO/InterfaceFile.h:336-338
+  /// Add a library for inlining to top level library 
+  ///
+  ///\param Document The library to inline with top level library 
----------------
Nit here and below in comments - missing trailing full stops.


================
Comment at: llvm/include/llvm/TextAPI/MachO/InterfaceFile.h:350
+
+
   /// Add a symbol to the symbols list or extend an existing one.
----------------
Nit: too many blank lines


================
Comment at: llvm/unittests/TextAPI/TextStubV4Tests.cpp:477
+  raw_svector_ostream OS(Buffer);
+  auto Result = TextAPIWriter::writeToStream(OS, File);
+  EXPECT_FALSE(Result);
----------------
There's a lot of usage of `auto` in this and the other files, and I'm pretty sure this is stretching LLVM's coding standard on the topic - the type isn't obvious. See https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67646





More information about the llvm-commits mailing list