[PATCH] D69131: [NFC][XCOFF] Using crtp to refactor the xcoff section header

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM with minor comments.



================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:50
 
-struct XCOFFSectionHeader32 {
+template <typename T> struct XCOFFSetionHeader {
+  // Least significant 3 bits are reserved.
----------------
Typo: s/Setion/Section/;
Since the base class does not represent a section header by itself, name this something like `XCOFFSectionHeaderBase`.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:49
 
+// Explict instantiate template class.
+template struct XCOFFSetionHeader<XCOFFSectionHeader32>;
----------------
s/Explict/Explicitly/;
s/class/classes/;


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:54
+template <typename T> StringRef XCOFFSetionHeader<T>::getName() const {
+  const T &DerivedXCOFFSetionHeader = static_cast<const T &>(*this);
+  return generateXCOFFFixedNameStringRef(DerivedXCOFFSetionHeader.Name);
----------------
Typo: s/Setion/Section/;


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:59
+template <typename T> uint16_t XCOFFSetionHeader<T>::getSectionType() const {
+  const T &DerivedXCOFFSetionHeader = static_cast<const T &>(*this);
+  return DerivedXCOFFSetionHeader.Flags & SectionFlagsTypeMask;
----------------
Same comment.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D69131