Zhanyong Wan (λx.x x)
2010-Nov-29 19:07 UTC
[LLVMdev] [cfe-dev] draft rule for naming types/functions/variables
On Sun, Nov 28, 2010 at 11:02 PM, Anton Korobeynikov <anton at korobeynikov.info> wrote:>> I enjoyed the new coding style in recent patches. Camel case makes it easy >> to pick a descriptive name. Starting functions and variables with lower >> cases reduces chances to conflict with a type name. > Honestly speaking, I don't. Especially in the cases when varname is > made from an > acronym. E.g. MachineInstr *MI looks much better than MachineInstr *mi, etc. > > See latest Rafael's patch as an example.I think it's worthwhile to note that: C++ is such a complex language that no single naming convention will be able to cover all cases. Therefore we aim at a reasonable default that works for the majority of the cases. As Chris put at the beginning of the coding standards, "no coding standards should be regarded as absolute requirements to be followed in all instances." Exceptions can be made (although rarely) when truly needed. Csaba has pointed out the problem of possible clash (at least confusion) with macro names for variable names like MI. I agree that 'mi' is not a great name either in this case. In fact, it's not even camel case. How about instr? Or, perhaps a name that clarifies its role (jumpInstr, nextInstr, etc)? Or, if this is a short-lived local variable, I'd be fine with 'mi'. Cheers, -- Zhanyong
Chris Lattner
2010-Nov-30 18:56 UTC
[LLVMdev] [cfe-dev] draft rule for naming types/functions/variables
On Nov 29, 2010, at 11:07 AM, Zhanyong Wan (λx.x x) wrote:> C++ is such a complex language that no single naming convention will > be able to cover all cases. Therefore we aim at a reasonable default > that works for the majority of the cases. As Chris put at the > beginning of the coding standards, "no coding standards should be > regarded as absolute requirements to be followed in all instances."Completely agreed.> Exceptions can be made (although rarely) when truly needed.Wait, but the coding standards document also says not to use exceptions! <sorry bad, joke> :)> Csaba has pointed out the problem of possible clash (at least > confusion) with macro names for variable names like MI. I agree thatI don't think that conflict with macros should be worried about. We've had our own problems with macros (e.g. system headers that define things like "i386") but there are straight-forward ways to deal with this. There is no need to live life in fear :)>> For capitalization, I generally prefer capital names with the exception being one character names that are often metasyntactic names (like i/j). > > If possible, I'd prefer that all variable names have the same style. > I'm afraid that we'll end up with the current inconsistent style if we > leave it to people to interpret whether a name is metasyntactic and > thus should be lower-case.Yes, I see where you're coming from. On the other hand, I don't see that standardizing the names of local variables is particularly useful. My viewpoint of standardizing naming conventions is that it makes it obvious for *clients of APIs* and prevents "having to look it up or think about it" when you're interacting with various subsystems. Local variables inherently cannot be used by other subsystems. If you're working on the body of a function, it is reasonable (and not usually difficult) to figure out what is going on in it. In cases where it is difficult, the capitalization of a variable isn't going to make it easier.>From this perspective, I see that it is most important to standardize the naming of: 1) types, 2) methods, 3) public instance variables, 4) enumerators. If you agree, then we don't have to argue about/standardize "Type type" or "MachineInstr *MI" vs "mi". It also means far less churn when someone decides to go crazy fixing the codebase. :)Given that, here are some suggestions for a future rev of your patch: +<p>In general, names of types, functions, and variables should be in +camel case (e.g. <tt>TextFileReader</tt> and <tt>isLValue</tt>). I'd suggest using <tt>isLValue()</tt> to make it clear that one is a type and one is a function. +Function names should be verb phrases I completely agree, but it is worth adding a short justification, explaining that methods are "actions". +Variable (including function parameter) names should start with a lower-case letter. Please remove this. In its place, we should have a naming rule for public ivars, and for enumerations and enumerators. I'd suggest ivars and enums follow the same rules as types. If there is a doubt, the enum should be a "Kind", e.g. "ValueTy" in llvm/Value.h should be "ValueKind". Enumerators (unless they are in their own small namespace) should have a prefix. For example, the enumerators in the ValueTy enum should be VK_Argument, VK_BasicBlock, etc. These rules shouldn't apply to enums that are just convenience constants, like: enum { MaxSize = 42, Density = 12 }; These should follow the same rules as ivars if they are members of classes. Thank you for driving this forward Zhanyong, I think that this will make the public APIs much more consistent and predictable! -Chris
Francois Pichet
2010-Nov-30 19:12 UTC
[LLVMdev] [cfe-dev] draft rule for naming types/functions/variables
2010/11/30 Chris Lattner <clattner at apple.com>:> Wait, but the coding standards document also says not to use exceptions! <sorry bad, joke> :)No it doesn't. It would be good to explicitly add that rule.
Zhanyong Wan (λx.x x)
2010-Dec-01 01:02 UTC
[LLVMdev] [cfe-dev] draft rule for naming types/functions/variables
Thanks for the comments, Chris! Glad that we are making progress. I'll make most of the edits you suggested later today. Before that, there are a couple of high-level points I'd like to go over with you. 1. I totally agree that the biggest benefit of a naming convention is uniform APIs. On the other hand, an inconsistent local naming style hurts the productivity of contributors and can lead to bugs by causing confusion. I'm fine with local variables having short names when their scope is limited (say, fits within one screen) or their role is obvious (like 'i' for an iterator), but I do find the context switching annoying when I have to adjust for the different styles as I move to different parts of the codebase. Just something for you to consider. 2. (more important than #1) I'd like to understand the reason behind your preference for UpperCase names for ivars. Is it just a personal preference or is there a more profound reason? So far, I've heard that some people like lowerCase ivars (clear distinction from types, etc), and some people don't think that helps much. However, I'm yet to hear why UpperCase ivars are considered *better* than lowerCase, so I'm curious. Thanks. -- Zhanyong
Possibly Parallel Threads
- [LLVMdev] [cfe-dev] draft rule for naming types/functions/variables
- [LLVMdev] [cfe-dev] draft rule for naming types/functions/variables
- [LLVMdev] [cfe-dev] draft rule for naming types/functions/variables
- [LLVMdev] [cfe-dev] draft rule for naming types/functions/variables
- [LLVMdev] draft rule for naming types/functions/variables