[PATCH] D57896: Variable names rule

Chris Lattner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 19 11:39:41 PDT 2019


lattner added a comment.

In D57896#1434877 <https://reviews.llvm.org/D57896#1434877>, @Charusso wrote:

> static Optional<const llvm::APSInt *>
>  getConcreteIntegerValue(const Expr *CondVarExpr, const ExplodedNode *N) {
>  //...
>
>   if (const auto *DRE = dyn_cast_or_null<DeclRefExpr>(CondVarExpr)) {
>     if (const auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
>
> //...
>  }
>
>   would be:
>  
>
>
> static Optional<const llvm::APSInt *>                                           |
>  getConcreteIntegerValue(const Expr *cond_var_expr, const ExplodedNode *node) {  |
>  //...                                                                           |
>
>   if (const auto *decl_ref_expr = dyn_cast_or_null<DeclRefExpr>(cond_var_expr)) {
>     if (const auto *var_decl = dyn_cast_or_null<VarDecl>(decl_ref_expr->getDecl())) {
>
> //...                                                                           |
>  }                                                             whoops column-81 ~^
>
>   Hungarian notation on members and globals are cool idea. However, the notation is made without the `_` part, so I think `mMember` is better than `m_member` as we used to 80-column standard and it is waste of space and hurts your C-developer eyes. I would recommend `b` prefix to booleans as Unreal Engine 4 styling is used to do that (`bIsCoolStyle`) and it is handy. It is useful because booleans usually has multiple prefixes: `has, have, is` and you would list all the booleans together in autocompletion. Yes, there is a problem: if the notation is not capital like the pure Hungarian notation then it is problematic to list and we are back to the `BIsCapitalLetter` and `MMember` CamelCase-world where we started (except one project). I think @lattner could say if it is useful as all the Apple projects based on those notations and could be annoying.


FWIW, my suggestion is *not* to expand names like DRE to decl_ref_expr, I agree that doesn't add clarity to the code.  Two possibilities: "dre", or "decl" which is what I would write today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896





More information about the cfe-commits mailing list