[PATCH] D68062: Propeller lld framework for basicblock sections

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 3 22:14:06 PDT 2019


ruiu added inline comments.


================
Comment at: lld/ELF/Propeller.cpp:133
+        SOrdinal == 0) {
+      reportProfError("Invalid ordinal field.");
+      return false;
----------------
Please follow the local convention -- if in doubt, please read other files to find out what we are doing there. Error messages shouldn't start with an uppercase letter, and shouldn't end with a full stop.


================
Comment at: lld/ELF/Propeller.h:257
+
+  void reportProfError(const StringRef msg) const;
+
----------------
You should avoid using `Prop` or `Propeller` as a part of an identifier for Propeller, because essentially everything you are writing is for Propeller. So it's not very explanatory. For example, I'd name this reportParseError.

Please remove `const` from `const StringRef`.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D68062





More information about the llvm-commits mailing list