michaelplatings updated this revision to Diff 187339.
michaelplatings added a comment.
Changed recommendation for acronyms from lower case to upper case, as suggested
by several responses to the RFC.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57896/new/
https://reviews.llvm.org/D57896
Files:
.clang-tidy
clang/.clang-tidy
llvm/.clang-tidy
llvm/docs/CodingStandards.rst
Index: llvm/docs/CodingStandards.rst
===================================================================
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -311,13 +311,13 @@
.. code-block:: c++
- Object.emitName(nullptr);
+ object.emitName(nullptr);
An in-line C-style comment makes the intent obvious:
.. code-block:: c++
- Object.emitName(/*Prefix=*/nullptr);
+ object.emitName(/*prefix=*/nullptr);
Commenting out large blocks of code is discouraged, but if you really have to do
this (for documentation purposes or as a suggestion for debug printing), use
@@ -355,8 +355,8 @@
.. code-block:: c++
- /// Sets the xyzzy property to \p Baz.
- void setXyzzy(bool Baz);
+ /// Sets the xyzzy property to \p baz.
+ void setXyzzy(bool baz);
A documentation comment that uses all Doxygen features in a preferred way:
@@ -364,18 +364,18 @@
/// Does foo and bar.
///
- /// Does not do foo the usual way if \p Baz is true.
+ /// Does not do foo the usual way if \p baz is true.
///
/// Typical usage:
/// \code
- /// fooBar(false, "quux", Res);
+ /// fooBar(false, "quux", res);
/// \endcode
///
- /// \param Quux kind of foo to do.
+ /// \param quux kind of foo to do.
/// \param [out] Result filled with bar sequence on foo success.
///
/// \returns true on success.
- bool fooBar(bool Baz, StringRef Quux, std::vector<int> &Result);
+ bool fooBar(bool baz, StringRef quux, std::vector<int> &result);
Don't duplicate the documentation comment in the header file and in the
implementation file. Put the documentation comments for public APIs into the
@@ -603,7 +603,7 @@
foo({a, b, c}, {1, 2, 3});
- llvm::Constant *Mask[] = {
+ llvm::Constant *mask[] = {
llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 0),
llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 1),
llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 2)};
@@ -633,7 +633,7 @@
.. code-block:: c++
- if (V = getValue()) {
+ if (v = getValue()) {
...
}
@@ -644,7 +644,7 @@
.. code-block:: c++
- if ((V = getValue())) {
+ if ((v = getValue())) {
...
}
@@ -736,7 +736,7 @@
class Foo;
// Breaks mangling in MSVC.
- struct Foo { int Data; };
+ struct Foo { int data; };
* As a rule of thumb, ``struct`` should be kept to structures where *all*
members are declared public.
@@ -746,17 +746,17 @@
// Foo feels like a class... this is strange.
struct Foo {
private:
- int Data;
+ int data;
public:
- Foo() : Data(0) { }
- int getData() const { return Data; }
- void setData(int D) { Data = D; }
+ Foo() : data(0) { }
+ int getData() const { return data; }
+ void setData(int d) { data = d; }
};
// Bar isn't POD, but it does look like a struct.
struct Bar {
- int Data;
- Bar() : Data(0) { }
+ int data;
+ Bar() : data(0) { }
};
Do not use Braced Initializer Lists to Call a Constructor
@@ -780,7 +780,7 @@
Foo(std::string filename);
// Construct a Foo by looking up the Nth element of some global data ...
- Foo(int N);
+ Foo(int n);
// ...
};
@@ -789,7 +789,7 @@
std::fill(foo.begin(), foo.end(), Foo("name"));
// The pair is just being constructed like an aggregate, use braces.
- bar_map.insert({my_key, my_value});
+ bar_map.insert({myKey, myValue});
If you use a braced initializer list when initializing a variable, use an equals before the open curly brace:
@@ -821,15 +821,15 @@
.. code-block:: c++
// Typically there's no reason to copy.
- for (const auto &Val : Container) { observe(Val); }
- for (auto &Val : Container) { Val.change(); }
+ for (const auto &val : container) { observe(val); }
+ for (auto &val : container) { val.change(); }
// Remove the reference if you really want a new copy.
- for (auto Val : Container) { Val.change(); saveSomewhere(Val); }
+ for (auto val : container) { val.change(); saveSomewhere(val); }
// Copy pointers, but make it clear that they're pointers.
- for (const auto *Ptr : Container) { observe(*Ptr); }
- for (auto *Ptr : Container) { Ptr->change(); }
+ for (const auto *ptr : container) { observe(*ptr); }
+ for (auto *ptr : container) { ptr->change(); }
Beware of non-determinism due to ordering of pointers
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -968,9 +968,9 @@
.. code-block:: c++
- Value *doSomething(Instruction *I) {
- if (!I->isTerminator() &&
- I->hasOneUse() && doOtherThing(I)) {
+ Value *doSomething(Instruction *instruction) {
+ if (!instruction->isTerminator() &&
+ instruction->hasOneUse() && doOtherThing(instruction)) {
... some long code ....
}
@@ -992,18 +992,18 @@
.. code-block:: c++
- Value *doSomething(Instruction *I) {
+ Value *doSomething(Instruction *instruction) {
// Terminators never need 'something' done to them because ...
- if (I->isTerminator())
+ if (instruction->isTerminator())
return 0;
// We conservatively avoid transforming instructions with multiple uses
// because goats like cheese.
- if (!I->hasOneUse())
+ if (!instruction->hasOneUse())
return 0;
// This is really just here for example.
- if (!doOtherThing(I))
+ if (!doOtherThing(instruction))
return 0;
... some long code ....
@@ -1014,8 +1014,8 @@
.. code-block:: c++
- for (Instruction &I : BB) {
- if (auto *BO = dyn_cast<BinaryOperator>(&I)) {
+ for (Instruction &instruction : BB) {
+ if (auto *BO = dyn_cast<BinaryOperator>(&instruction)) {
Value *LHS = BO->getOperand(0);
Value *RHS = BO->getOperand(1);
if (LHS != RHS) {
@@ -1034,8 +1034,8 @@
.. code-block:: c++
- for (Instruction &I : BB) {
- auto *BO = dyn_cast<BinaryOperator>(&I);
+ for (Instruction &instruction : BB) {
+ auto *BO = dyn_cast<BinaryOperator>(&instruction);
if (!BO) continue;
Value *LHS = BO->getOperand(0);
@@ -1062,18 +1062,18 @@
.. code-block:: c++
case 'J': {
- if (Signed) {
- Type = Context.getsigjmp_bufType();
- if (Type.isNull()) {
- Error = ASTContext::GE_Missing_sigjmp_buf;
+ if (signed) {
+ type = context.getsigjmp_bufType();
+ if (type.isNull()) {
+ error = ASTContext::GE_Missing_sigjmp_buf;
return QualType();
} else {
break;
}
} else {
- Type = Context.getjmp_bufType();
- if (Type.isNull()) {
- Error = ASTContext::GE_Missing_jmp_buf;
+ type = context.getjmp_bufType();
+ if (type.isNull()) {
+ error = ASTContext::GE_Missing_jmp_buf;
return QualType();
} else {
break;
@@ -1086,16 +1086,16 @@
.. code-block:: c++
case 'J':
- if (Signed) {
- Type = Context.getsigjmp_bufType();
- if (Type.isNull()) {
- Error = ASTContext::GE_Missing_sigjmp_buf;
+ if (signed) {
+ type = context.getsigjmp_bufType();
+ if (type.isNull()) {
+ error = ASTContext::GE_Missing_sigjmp_buf;
return QualType();
}
} else {
- Type = Context.getjmp_bufType();
- if (Type.isNull()) {
- Error = ASTContext::GE_Missing_jmp_buf;
+ type = context.getjmp_bufType();
+ if (type.isNull()) {
+ error = ASTContext::GE_Missing_jmp_buf;
return QualType();
}
}
@@ -1106,13 +1106,13 @@
.. code-block:: c++
case 'J':
- if (Signed)
- Type = Context.getsigjmp_bufType();
+ if (signed)
+ type = context.getsigjmp_bufType();
else
- Type = Context.getjmp_bufType();
+ type = context.getjmp_bufType();
- if (Type.isNull()) {
- Error = Signed ? ASTContext::GE_Missing_sigjmp_buf :
+ if (type.isNull()) {
+ error = signed ? ASTContext::GE_Missing_sigjmp_buf :
ASTContext::GE_Missing_jmp_buf;
return QualType();
}
@@ -1130,14 +1130,14 @@
.. code-block:: c++
- bool FoundFoo = false;
- for (unsigned I = 0, E = BarList.size(); I != E; ++I)
- if (BarList[I]->isFoo()) {
- FoundFoo = true;
+ bool foundFoo = false;
+ for (unsigned i = 0, end = barList.size(); i != end; ++i)
+ if (barList[i]->isFoo()) {
+ foundFoo = true;
break;
}
- if (FoundFoo) {
+ if (foundFoo) {
...
}
@@ -1149,15 +1149,15 @@
.. code-block:: c++
/// \returns true if the specified list has an element that is a foo.
- static bool containsFoo(const std::vector<Bar*> &List) {
- for (unsigned I = 0, E = List.size(); I != E; ++I)
- if (List[I]->isFoo())
+ static bool containsFoo(const std::vector<Bar*> &list) {
+ for (unsigned i = 0, end = list.size(); i != end; ++i)
+ if (list[i]->isFoo())
return true;
return false;
}
...
- if (containsFoo(BarList)) {
+ if (containsFoo(barList)) {
...
}
@@ -1191,9 +1191,15 @@
nouns and start with an upper-case letter (e.g. ``TextFileReader``).
* **Variable names** should be nouns (as they represent state). The name should
- be camel case, and start with an upper case letter (e.g. ``Leader`` or
- ``Boats``).
-
+ be camel case, and start with a lower case letter (e.g. ``leader`` or
+ ``boats``). Acronyms should be avoided unless well known, but if used they
+ should typically be upper case (e.g. ``TLI``).
+ It is acceptable, but not necessary, to use ``UpperCamelCase`` variable names
+ for consistency with existing code. If a code change renames an existing
+ variable, affecting multiple lines that wouldn't otherwise be touched by the
+ change, typically it is preferred to do the renaming in a separate patch to
+ keep the intent of functional changes clear.
+
* **Function names** should be verb phrases (as they represent actions), and
command-like function should be imperative. The name should be camel case,
and start with a lower case letter (e.g. ``openFile()`` or ``isFoo()``).
@@ -1232,16 +1238,22 @@
class VehicleMaker {
...
- Factory<Tire> F; // Bad -- abbreviation and non-descriptive.
- Factory<Tire> Factory; // Better.
- Factory<Tire> TireFactory; // Even better -- if VehicleMaker has more than one
- // kind of factories.
+ Factory<Tire> f; // Bad -- abbreviation and non-descriptive.
+ Factory<Tire> factory; // Better.
+ Factory<Tire> tireFactory; // Even better -- if VehicleMaker has more than
+ // one kind of factory.
};
- Vehicle makeVehicle(VehicleType Type) {
- VehicleMaker M; // Might be OK if having a short life-span.
- Tire Tmp1 = M.makeTire(); // Bad -- 'Tmp1' provides no information.
- Light Headlight = M.makeLight("head"); // Good -- descriptive.
+ Vehicle makeVehicle(VehicleType type) {
+ // Reusing the type name in lowerCamelCase form is often a good way to get
+ // a suitable variable name.
+ VehicleMaker vehicleMaker;
+
+ // Bad -- 'tmp1' provides no information.
+ Tire tmp1 = vehicleMaker.makeTire();
+
+ // Good -- descriptive.
+ Light headlight = vehicleMaker.makeLight("head");
...
}
@@ -1261,24 +1273,24 @@
.. code-block:: c++
- inline Value *getOperand(unsigned I) {
- assert(I < Operands.size() && "getOperand() out of range!");
- return Operands[I];
+ inline Value *getOperand(unsigned i) {
+ assert(i < operands.size() && "getOperand() out of range!");
+ return operands[I];
}
Here are more examples:
.. code-block:: c++
- assert(Ty->isPointerType() && "Can't allocate a non-pointer type!");
+ assert(type->isPointerType() && "Can't allocate a non-pointer type!");
- assert((Opcode == Shl || Opcode == Shr) && "ShiftInst Opcode invalid!");
+ assert((opcode == SHL || opcode == SHR) && "ShiftInst Opcode invalid!");
assert(idx < getNumSuccessors() && "Successor # out of range!");
- assert(V1.getType() == V2.getType() && "Constant types must be identical!");
+ assert(v1.getType() == v2.getType() && "Constant types must be identical!");
- assert(isa<PHINode>(Succ->front()) && "Only works on PHId BBs!");
+ assert(isa<PHINode>(succ->front()) && "Only works on PHId BBs!");
You get the idea.
@@ -1316,11 +1328,11 @@
.. code-block:: c++
- unsigned Size = V.size();
- assert(Size > 42 && "Vector smaller than it should be");
+ unsigned size = v.size();
+ assert(size > 42 && "Vector smaller than it should be");
- bool NewToSet = Myset.insert(Value);
- assert(NewToSet && "The value shouldn't be in the set yet");
+ bool newToSet = myset.insert(Value);
+ assert(newToSet && "The value shouldn't be in the set yet");
These are two interesting different cases. In the first case, the call to
``V.size()`` is only useful for the assert, and we don't want it executed when
@@ -1332,10 +1344,10 @@
.. code-block:: c++
- assert(V.size() > 42 && "Vector smaller than it should be");
+ assert(v.size() > 42 && "Vector smaller than it should be");
- bool NewToSet = Myset.insert(Value); (void)NewToSet;
- assert(NewToSet && "The value shouldn't be in the set yet");
+ bool newToSet = myset.insert(value); (void)newToSet;
+ assert(newToSet && "The value shouldn't be in the set yet");
Do Not Use ``using namespace std``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -1405,8 +1417,8 @@
.. code-block:: c++
BasicBlock *BB = ...
- for (Instruction &I : *BB)
- ... use I ...
+ for (Instruction &instruction : *BB)
+ ... use instruction ...
Don't evaluate ``end()`` every time through a loop
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -1419,8 +1431,8 @@
.. code-block:: c++
BasicBlock *BB = ...
- for (auto I = BB->begin(); I != BB->end(); ++I)
- ... use I ...
+ for (auto i = BB->begin(); i != BB->end(); ++i)
+ ... use i ...
The problem with this construct is that it evaluates "``BB->end()``" every time
through the loop. Instead of writing the loop like this, we strongly prefer
@@ -1430,8 +1442,8 @@
.. code-block:: c++
BasicBlock *BB = ...
- for (auto I = BB->begin(), E = BB->end(); I != E; ++I)
- ... use I ...
+ for (auto i = BB->begin(), end = BB->end(); i != end; ++i)
+ ... use i ...
The observant may quickly point out that these two loops may have different
semantics: if the container (a basic block in this case) is being mutated, then
@@ -1445,7 +1457,7 @@
start of the loop. In this case, the cost is probably minor --- a few extra
loads every time through the loop. However, if the base expression is more
complex, then the cost can rise quickly. I've seen loops where the end
-expression was actually something like: "``SomeMap[X]->end()``" and map lookups
+expression was actually something like: "``someMap[x]->end()``" and map lookups
really aren't cheap. By writing it in the second form consistently, you
eliminate the issue entirely and don't even have to think about it.
@@ -1549,27 +1561,27 @@
.. code-block:: c++
- if (X) ...
- for (I = 0; I != 100; ++I) ...
- while (LLVMRocks) ...
+ if (x) ...
+ for (i = 0; i != 100; ++i) ...
+ while (llvmRocks) ...
somefunc(42);
assert(3 != 4 && "laws of math are failing me");
- A = foo(42, 92) + bar(X);
+ a = foo(42, 92) + bar(X);
and this is bad:
.. code-block:: c++
- if(X) ...
- for(I = 0; I != 100; ++I) ...
- while(LLVMRocks) ...
+ if(x) ...
+ for(i = 0; i != 100; ++i) ...
+ while(llvmRocks) ...
somefunc (42);
assert (3 != 4 && "laws of math are failing me");
- A = foo (42, 92) + bar (X);
+ a = foo (42, 92) + bar (X);
The reason for doing this is not completely arbitrary. This style makes control
flow operators stand out more, and makes expressions flow better. The function
@@ -1581,7 +1593,7 @@
.. code-block:: c++
- A = foo ((42, 92) + bar) (X);
+ a = foo ((42, 92) + bar) (x);
when skimming through the code. By avoiding a space in a function, we avoid
this misinterpretation.
@@ -1589,8 +1601,8 @@
Prefer Preincrement
^^^^^^^^^^^^^^^^^^^
-Hard fast rule: Preincrement (``++X``) may be no slower than postincrement
-(``X++``) and could very well be a lot faster than it. Use preincrementation
+Hard fast rule: Preincrement (``++x``) may be no slower than postincrement
+(``x++``) and could very well be a lot faster than it. Use preincrementation
whenever possible.
The semantics of postincrement include making a copy of the value being
Index: llvm/.clang-tidy
===================================================================
--- llvm/.clang-tidy
+++ llvm/.clang-tidy
@@ -7,11 +7,11 @@
- key: readability-identifier-naming.FunctionCase
value: camelBack
- key: readability-identifier-naming.MemberCase
- value: CamelCase
+ value: aNy_CasE
- key: readability-identifier-naming.ParameterCase
- value: CamelCase
+ value: aNy_CasE
- key: readability-identifier-naming.UnionCase
value: CamelCase
- key: readability-identifier-naming.VariableCase
- value: CamelCase
+ value: aNy_CasE
Index: clang/.clang-tidy
===================================================================
--- clang/.clang-tidy
+++ clang/.clang-tidy
@@ -12,11 +12,11 @@
- key: readability-identifier-naming.FunctionCase
value: camelBack
- key: readability-identifier-naming.MemberCase
- value: CamelCase
+ value: aNy_CasE
- key: readability-identifier-naming.ParameterCase
- value: CamelCase
+ value: aNy_CasE
- key: readability-identifier-naming.UnionCase
value: CamelCase
- key: readability-identifier-naming.VariableCase
- value: CamelCase
+ value: aNy_CasE
Index: .clang-tidy
===================================================================
--- .clang-tidy
+++ .clang-tidy
@@ -7,11 +7,11 @@
- key: readability-identifier-naming.FunctionCase
value: camelBack
- key: readability-identifier-naming.MemberCase
- value: CamelCase
+ value: aNy_CasE
- key: readability-identifier-naming.ParameterCase
- value: CamelCase
+ value: aNy_CasE
- key: readability-identifier-naming.UnionCase
value: CamelCase
- key: readability-identifier-naming.VariableCase
- value: CamelCase
+ value: aNy_CasE
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
