[PATCH] D69354: Make coding standards document more inclusive

BĂ©atrice Creusillet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 30 01:35:33 PDT 2019


bmcreusillet added inline comments.


================
Comment at: llvm/docs/CodingStandards.rst:79
+<https://github.com/llvm/llvm-project/tree/master/llvm/include/llvm/ADT>`_)
+providing func tionality missing from the standard library for which there are
+standard interfaces or active work on adding standard interfaces will often be
----------------
There seem to be a spurious space in "func tionality"

Also the whole sentence is somewhat long, I would add commas for instance before "providing" and before "will often" to ease readability.



================
Comment at: llvm/docs/CodingStandards.rst:170
 
-Classes are one fundamental part of a good object oriented design.  As such, a
+Classes are a fundamental part of a object-oriented design.  As such, a
 class definition should have a comment block that explains what the class is
----------------
a object-oriented -> an object oriented?


================
Comment at: llvm/docs/CodingStandards.rst:184
 Good things to talk about here are what happens when something unexpected
-happens: does the method return null?  Abort?  Format your hard disk?
+happens: does the method return null?
 
----------------
I would turn this into: "for instance, does the method return null?", otherwise it may seem the only possibility. 


================
Comment at: llvm/docs/CodingStandards.rst:294
 
-  // In Something.cpp:
+  // In Example.cpp:
 
----------------
The "In" does not seem consistent with the C example


================
Comment at: llvm/docs/CodingStandards.rst:308
 
-Consider:
+  // In Example.cpp:
 
----------------
same remark about "In"


================
Comment at: llvm/docs/CodingStandards.rst:426
 
 With C++11, there are significantly more uses of braced lists to perform
+initialization. For example, they can be used to construct aggregate
----------------
A remark not related to the changes themselves:

"With C++11": Shouldn't it be "From C++11" or something similar?


================
Comment at: llvm/docs/CodingStandards.rst:507
+
+Globals in different source files are initialized in `arbitrary order
+<https://yosefk.com/c++fqa/ctors.html#fqa-10.12>`.
----------------
This sentence does not appear to be properly linked to the preceding nor to the following one.

I feel a small explanation of the consequences in this sentence could be helpful. Also beginning the next sentence with "Moreover" or something appropriate.


================
Comment at: llvm/docs/CodingStandards.rst:510
+
+Static constructors have have negative impact on launch time of programs that
+use LLVM as a library. We would really like for there to be zero cost for
----------------
s/have have/have/


================
Comment at: llvm/docs/CodingStandards.rst:559
 
 In C++11 there is a "generalized initialization syntax" which allows calling
 constructors using braced initializer lists. Do not use these to call
----------------
Same remarks as before about the reference to C++11 ?


================
Comment at: llvm/docs/CodingStandards.rst:941
+Instead of this sort of loop, we prefer to use a predicate function (which may
+be `static`_) that uses `early exits`_.  We prefer the code to be structured
+like this:
----------------
"we prefer" is repeated at a short interval. Maybe change the second one into "The code should instead be structured like this:"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69354





More information about the llvm-commits mailing list