michaelplatings updated this revision to Diff 187252.
michaelplatings added a comment.
Update .clang-tidy files to use aNy_CasE until camelBackOrCase is available.
Add more guidance around acronyms.
Add more guidance around consistency with existing CamelCase variable names.
Change other code examples to camelBack.
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
@@ -568,16 +568,16 @@
.. code-block:: c++
dyn_switch(V->stripPointerCasts(),
- [] (PHINode *PN) {
+ [] (PHINode *phiNode) {
// process phis...
},
- [] (SelectInst *SI) {
+ [] (SelectInst *selectInst) {
// process selects...
},
- [] (LoadInst *LI) {
+ [] (LoadInst *loadInst) {
// process loads...
},
- [] (AllocaInst *AI) {
+ [] (AllocaInst *allocaInst) {
// process allocas...
});
@@ -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,11 +1014,11 @@
.. code-block:: c++
- for (Instruction &I : BB) {
- if (auto *BO = dyn_cast<BinaryOperator>(&I)) {
- Value *LHS = BO->getOperand(0);
- Value *RHS = BO->getOperand(1);
- if (LHS != RHS) {
+ for (Instruction &instruction : bb) {
+ if (auto *binaryOperator = dyn_cast<BinaryOperator>(&instruction)) {
+ Value *lhs = binaryOperator->getOperand(0);
+ Value *rhs = binaryOperator->getOperand(1);
+ if (lhs != rhs) {
...
}
}
@@ -1034,13 +1034,13 @@
.. code-block:: c++
- for (Instruction &I : BB) {
- auto *BO = dyn_cast<BinaryOperator>(&I);
- if (!BO) continue;
+ for (Instruction &instruction : bb) {
+ auto *binaryOperator = dyn_cast<BinaryOperator>(&instruction);
+ if (!binaryOperator) continue;
- Value *LHS = BO->getOperand(0);
- Value *RHS = BO->getOperand(1);
- if (LHS == RHS) continue;
+ Value *lhs = binaryOperator->getOperand(0);
+ Value *rhs = binaryOperator->getOperand(1);
+ if (lhs == rhs) continue;
...
}
@@ -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 be lower case (e.g. ``rhs``).
+ 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 basic blocks!");
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``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -1404,9 +1416,9 @@
.. code-block:: c++
- BasicBlock *BB = ...
- for (Instruction &I : *BB)
- ... use I ...
+ BasicBlock *bb = ...
+ for (Instruction &instruction : *bb)
+ ... use instruction ...
Don't evaluate ``end()`` every time through a loop
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -1418,24 +1430,24 @@
.. code-block:: c++
- BasicBlock *BB = ...
- for (auto I = BB->begin(); I != BB->end(); ++I)
- ... use I ...
+ BasicBlock *bb = ...
+ for (auto i = bb->begin(); i != bb->end(); ++i)
+ ... use i ...
-The problem with this construct is that it evaluates "``BB->end()``" every time
+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
loops to be written so that they evaluate it once before the loop starts. A
convenient way to do this is like so:
.. code-block:: c++
- BasicBlock *BB = ...
- for (auto I = BB->begin(), E = BB->end(); I != E; ++I)
- ... use I ...
+ BasicBlock *bb = ...
+ 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
-"``BB->end()``" may change its value every time through the loop and the second
+"``bb->end()``" may change its value every time through the loop and the second
loop may not in fact be correct. If you actually do depend on this behavior,
please write the loop in the first form and add a comment indicating that you
did it intentionally.
@@ -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
@@ -1669,7 +1681,7 @@
...
public:
StringSort(...)
- bool operator<(const char *RHS) const;
+ bool operator<(const char *rhs) const;
};
} // end anonymous namespace
@@ -1677,7 +1689,7 @@
...
}
- bool StringSort::operator<(const char *RHS) const {
+ bool StringSort::operator<(const char *rhs) const {
...
}
@@ -1691,14 +1703,14 @@
...
public:
StringSort(...)
- bool operator<(const char *RHS) const;
+ bool operator<(const char *rhs) const;
};
void runHelper() {
...
}
- bool StringSort::operator<(const char *RHS) const {
+ bool StringSort::operator<(const char *rhs) const {
...
}
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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
