aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM! ================ Comment at: docs/clang-tidy/checks/readability-isolate-declaration.rst:45 + +Global variables and member variables are excluded. + ---------------- JonasToth wrote: > aaron.ballman wrote: > > Why are global variables excluded from this check? It seems like they > > should have the exact same behavior as local variables in terms of the > > transformation, no? > I thought so too, but take a look at the AST: > > ``` > int global_i, global_j = 42, global_k; > void f() { > int local_i, local_j = 42, local_k; > } > ``` > > ``` > |-VarDecl 0x55c428c4ab08 > </home/jonas/Programme/test/test_decls_global.cpp:1:1, col:5> col:5 global_i > 'int' > |-VarDecl 0x55c428c4abc0 <col:1, col:26> col:15 global_j 'int' cinit > | `-IntegerLiteral 0x55c428c4ac20 <col:26> 'int' 42 > |-VarDecl 0x55c428c4ac58 <col:1, col:30> col:30 global_k 'int' > `-FunctionDecl 0x55c428c4ad30 <line:2:1, line:4:1> line:2:6 f 'void ()' > `-CompoundStmt 0x55c428c4af90 <col:10, line:4:1> > `-DeclStmt 0x55c428c4af78 <line:3:5, col:39> > |-VarDecl 0x55c428c4ade8 <col:5, col:9> col:9 local_i 'int' > |-VarDecl 0x55c428c4ae60 <col:5, col:28> col:18 local_j 'int' cinit > | `-IntegerLiteral 0x55c428c4aec0 <col:28> 'int' 42 > `-VarDecl 0x55c428c4aef8 <col:5, col:32> col:32 local_k 'int' > ``` > > The locals create a `DeclStmt` and the globals don't, right now the > transformation depends on the fact that its a `DeclStmt`, similar to class > members that are `FieldDecl`. > > I did short investigation on the issue, but couldn't think of a good way to > fix that issue. I was not capable of "grouping" these decls even though they > are together. Maybe I just missed something obvious, but right it's not > supported. I would love to actually support it tough. Yeah, this looks like a deficiency with the AST representation, to me. It doesn't need to be solved for this patch, but it may be an interesting next step for the check for a future patch. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits