[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