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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to