[PATCH] D53945: [TextAPI] TBD Reader/Writer

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

Oops. Those comments were for and old diff. The no-op sorts are gone, but the other three still apply.



================
Comment at: llvm/include/llvm/TextAPI/MachO/InterfaceFile.h:106
+  StringRef getInstallName() const { return InstallName; };
+  void setArchitectures(ArchitectureSet Archs) { Architectures |= Archs; }
+  ArchitectureSet getArchitectures() const { return Architectures; }
----------------
Should this be called "addArchitectures" instead, given that it's an additive operation?


================
Comment at: llvm/include/llvm/TextAPI/MachO/InterfaceFile.h:172
+  ///
+  /// This is used by the YAML writter to identify the specification it should.
+  ///
----------------
Looks like a partial comment?

And "writter" should be "writer".


================
Comment at: llvm/lib/TextAPI/MachO/TextStub.cpp:188-196
+inline Flags operator|(const Flags a, const Flags b) {
+  return static_cast<Flags>(static_cast<unsigned>(a) |
+                            static_cast<unsigned>(b));
+}
+
+inline Flags operator|=(Flags &a, const Flags b) {
+  a = static_cast<Flags>(static_cast<unsigned>(a) | static_cast<unsigned>(b));
----------------
You might be able to use llvm/ADT/BitmaskEnum.h here, rather than defining these operators yourself.


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

https://reviews.llvm.org/D53945