diff --git a/.clang-tidy b/.clang-tidy --- a/.clang-tidy +++ b/.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 diff --git a/clang/.clang-tidy b/clang/.clang-tidy --- a/clang/.clang-tidy +++ b/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 diff --git a/llvm/.clang-tidy b/llvm/.clang-tidy --- a/llvm/.clang-tidy +++ b/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 diff --git a/llvm/docs/CodingStandards.rst b/llvm/docs/CodingStandards.rst --- a/llvm/docs/CodingStandards.rst +++ b/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 &Result); + bool fooBar(bool baz, StringRef quux, std::vector &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(&I)) { + for (Instruction &instruction : BB) { + if (auto *BO = dyn_cast(&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(&I); + for (Instruction &instruction : BB) { + auto *BO = dyn_cast(&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 (isSigned) { + 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 (isSigned) { + 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 (isSigned) + 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 = isSigned ? 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 &List) { - for (unsigned I = 0, E = List.size(); I != E; ++I) - if (List[I]->isFoo()) + static bool containsFoo(const std::vector &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 F; // Bad -- abbreviation and non-descriptive. - Factory Factory; // Better. - Factory TireFactory; // Even better -- if VehicleMaker has more than one - // kind of factories. + Factory f; // Bad -- abbreviation and non-descriptive. + Factory factory; // Better. + Factory 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(Succ->front()) && "Only works on PHId BBs!"); + assert(isa(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