[PATCH] D59419: [XCOFF] Add functionality for parsing AIX XCOFF object files header .

Digger via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 1 08:47:29 PDT 2019


DiggerLin added inline comments.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:108
+} // namespace object
+} // namespace llvm
+#endif // LLVM_OBJECT_XCOFFOBJECTFILE_H
----------------
sfertile wrote:
> minor nit: add a blank line before the `endif`
done


================
Comment at: llvm/include/llvm/ObjectYAML/XCOFFYAML.h:19
+namespace llvm {
+
+namespace XCOFFYAML {
----------------
sfertile wrote:
> minor nit: no blank line between the 2 namespace openings.
deleted , thanks


================
Comment at: llvm/include/llvm/ObjectYAML/XCOFFYAML.h:49
+} // namespace yaml
+} // namespace llvm
+#endif // LLVM_OBJECTYAML_XCOFFYAML_H
----------------
sfertile wrote:
> blank again.
added a blank line , thanks


================
Comment at: llvm/lib/ObjectYAML/XCOFFYAML.cpp:17
+#include <cstdint>
+#include <vector>
+
----------------
hubert.reinterpretcast wrote:
> I am not seeing a need for `<cstdint>` or `<vector>`. At the same time, `<string.h>` should be added for `::memset`.
I deleted the 
-#include "llvm/ObjectYAML/YAML.h"
-#include "llvm/Support/YAMLTraits.h"
-#include <cstdint>
-#include <vector>


================
Comment at: llvm/lib/ObjectYAML/XCOFFYAML.cpp:20
+namespace llvm {
+
+namespace XCOFFYAML {
----------------
sfertile wrote:
> nit: remove white-space.
done


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

https://reviews.llvm.org/D59419





More information about the llvm-commits mailing list