[PATCH] D69354: Make coding standards document more inclusive

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

A few minor further tweaks.



================
Comment at: llvm/docs/CodingStandards.rst:56
 
-LLVM, Clang, and LLD are currently written using C++14 conforming code,
-although we restrict ourselves to features which are available in the major
-toolchains supported as host compilers. The LLDB project is even more
-aggressive in the set of host compilers supported and thus uses still more
-features. Regardless of the supported features, code is expected to (when
-reasonable) be standard, portable, and modern C++14 code. We avoid unnecessary
-vendor-specific extensions, etc.
+LLVM, Clang, LLDB, and LLD are written using standard C++14 code. Avoid
+unnecessary vendor-specific extensions.
----------------
Polly? Maybe its not important to list everything here.

Maybe this should say "Unless otherwise documented, LLVM subprojects are written ..."


================
Comment at: llvm/docs/CodingStandards.rst:60-61
+Nevertheless, we restrict ourselves to features which are available in the
+major toolchains supported as host compilers.  Those features are the
+intersection of those supported in the minimum requirements described in the
+:doc:`GettingStarted` page, section `Software`.
----------------
"*Those* features are the intersection of *those* supported ..."

Maybe: "Nevertheless, we restrict ourselves to the intersection of features which are available..." and then "We describe our host compiler support in the GettingStarted page..."?


================
Comment at: llvm/docs/CodingStandards.rst:356
+There must be some limit to the width of the code in
 order to reasonably allow developers to have multiple files side-by-side in
 windows on a modest display.  If you are going to pick a width limit, it is
----------------
s/reasonably// ?


================
Comment at: llvm/docs/CodingStandards.rst:459
 
-If your code has compiler warnings in it, something is wrong --- you aren't
-casting values correctly, you have "questionable" constructs in your code, or
-you are doing something legitimately wrong.  Compiler warnings can cover up
-legitimate errors in output and make dealing with a translation unit difficult.
-
-It is not possible to prevent all warnings from all compilers, nor is it
-desirable.  Instead, pick a standard compiler (like ``gcc``) that provides a
-good thorough set of warnings, and stick to it.  At least in the case of
-``gcc``, it is possible to work around any spurious errors by changing the
-syntax of the code slightly.  For example, a warning that annoys me occurs when
-I write code like this:
+Treat compiler warnings seriously, they are often useful. Those that are not
+useful, can be often suppressed with a small code change. For example, an
----------------
I think we can make this even better... Maybe:

"Compiler warnings are often useful and will help improve the code. Those that are not ..."


================
Comment at: llvm/docs/CodingStandards.rst:491-494
+``dynamic_cast<>``).  These two language features violate
 the general C++ principle of *"you only pay for what you use"*, causing
 executable bloat even if exceptions are never used in the code base, or if RTTI
 is never used for a class.  Because of this, we turn them off globally in the
----------------
Delete the entire sentence from "These two language features ... is never used for a class.". I don't think it is phrased in an objective or good way IMO.

It might be possible to rephrase it, but honestly I don't think its that useful and so i'd also probable delete the subsequent sentence and keep going as far as needed. I don't think we need a lot of detailed discussion here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69354