[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 9 10:13:41 PDT 2017


lebedev.ri added a comment.

In https://reviews.llvm.org/D32942#776915, @malcolm.parsons wrote:

> Does this check consider `else if` chains to be nesting?
>
>   void nesting() { // 1
>     if (true) { // 2
>        int j;
>     } else if (true) { // 2 or 3?
>        int j;
>     } else if (true) { // 2 or 4?
>        int j;
>     } else if (true) { // 2 or 5?
>        int j;
>     }
>   }
>


Yes, this check, like  does consider `else if` chains to be nesting, as you can see from the following output:

  $ ./bin/clang-tidy -checks=readability-function-size -config='{CheckOptions: [{key: readability-function-size.NestingThreshold, value: 2}]}' /tmp/if-nest.cpp
  Error while trying to load a compilation database:
  Could not auto-detect compilation database for file "/tmp/if-nest.cpp"
  No compilation database found in /tmp or any parent directory
  json-compilation-database: Error while opening JSON database: No such file or directory
  Running without flags.
  1 warning generated.
  /tmp/if-nest.cpp:1:6: warning: function 'nesting' exceeds recommended size/complexity thresholds [readability-function-size]
  void nesting() { // 1
       ^
  /tmp/if-nest.cpp:2:13: note: nesting level 3 starts here (threshold 2)
    if (true) { // 2
              ^
  /tmp/if-nest.cpp:4:10: note: nesting level 3 starts here (threshold 2)
    } else if (true) { // 2 or 3?
           ^

Which makes sense, since in AST, they are nested:

  $ clang -Xclang -ast-dump -fsyntax-only  /tmp/if-nest.cpp
  TranslationUnitDecl 0x560a93687b60 <<invalid sloc>> <invalid sloc>
  |-TypedefDecl 0x560a936880f0 <<invalid sloc>> <invalid sloc> implicit __int128_t '__int128'
  | `-BuiltinType 0x560a93687dd0 '__int128'
  |-TypedefDecl 0x560a93688160 <<invalid sloc>> <invalid sloc> implicit __uint128_t 'unsigned __int128'
  | `-BuiltinType 0x560a93687df0 'unsigned __int128'
  |-TypedefDecl 0x560a936884a8 <<invalid sloc>> <invalid sloc> implicit __NSConstantString 'struct __NSConstantString_tag'
  | `-RecordType 0x560a93688250 'struct __NSConstantString_tag'
  |   `-CXXRecord 0x560a936881b8 '__NSConstantString_tag'
  |-TypedefDecl 0x560a93688540 <<invalid sloc>> <invalid sloc> implicit __builtin_ms_va_list 'char *'
  | `-PointerType 0x560a93688500 'char *'
  |   `-BuiltinType 0x560a93687bf0 'char'
  |-TypedefDecl 0x560a936bd730 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list 'struct __va_list_tag [1]'
  | `-ConstantArrayType 0x560a93688820 'struct __va_list_tag [1]' 1 
  |   `-RecordType 0x560a93688630 'struct __va_list_tag'
  |     `-CXXRecord 0x560a93688598 '__va_list_tag'
  `-FunctionDecl 0x560a936bd7e0 </tmp/if-nest.cpp:1:1, line:11:1> line:1:6 nesting 'void (void)'
    `-CompoundStmt 0x560a936bdcb8 <col:16, line:11:1>
      `-IfStmt 0x560a936bdc80 <line:2:3, line:10:3>
        |-<<<NULL>>>
        |-<<<NULL>>>
        |-CXXBoolLiteralExpr 0x560a936bd8b8 <line:2:7> '_Bool' true
        |-CompoundStmt 0x560a936bd960 <col:13, line:4:3>
        | `-DeclStmt 0x560a936bd948 <line:3:6, col:11>
        |   `-VarDecl 0x560a936bd8e8 <col:6, col:10> col:10 j 'int'
        `-IfStmt 0x560a936bdc48 <line:4:10, line:10:3>
          |-<<<NULL>>>
          |-<<<NULL>>>
          |-CXXBoolLiteralExpr 0x560a936bd980 <line:4:14> '_Bool' true
          |-CompoundStmt 0x560a936bda28 <col:20, line:6:3>
          | `-DeclStmt 0x560a936bda10 <line:5:6, col:11>
          |   `-VarDecl 0x560a936bd9b0 <col:6, col:10> col:10 j 'int'
          `-IfStmt 0x560a936bdc10 <line:6:10, line:10:3>
            |-<<<NULL>>>
            |-<<<NULL>>>
            |-CXXBoolLiteralExpr 0x560a936bda48 <line:6:14> '_Bool' true
            |-CompoundStmt 0x560a936bdaf0 <col:20, line:8:3>
            | `-DeclStmt 0x560a936bdad8 <line:7:6, col:11>
            |   `-VarDecl 0x560a936bda78 <col:6, col:10> col:10 j 'int'
            `-IfStmt 0x560a936bdbd8 <line:8:10, line:10:3>
              |-<<<NULL>>>
              |-<<<NULL>>>
              |-CXXBoolLiteralExpr 0x560a936bdb10 <line:8:14> '_Bool' true
              |-CompoundStmt 0x560a936bdbb8 <col:20, line:10:3>
              | `-DeclStmt 0x560a936bdba0 <line:9:6, col:11>
              |   `-VarDecl 0x560a936bdb40 <col:6, col:10> col:10 j 'int'
              `-<<<NULL>>>

However what i'm not totally understanding right now is

  /tmp/if-nest.cpp:2:13: note: nesting level 3 starts here (threshold 2)
    if (true) { // 2
              ^

If we look at `clang-query` output, that *should* be second nesting level...

  $ clang-query-5.0 /tmp/if-nest.cpp
  Error while trying to load a compilation database:
  Could not auto-detect compilation database for file "/tmp/if-nest.cpp"
  No compilation database found in /tmp or any parent directory
  json-compilation-database: Error while opening JSON database: No such file or directory
  Running without flags.
  clang-query> match compoundStmt()
  
  Match #1:
  
  /tmp/if-nest.cpp:1:16: note: "root" binds here
  void nesting() { // 1
                 ^~~~~~
  
  Match #2:
  
  /tmp/if-nest.cpp:2:13: note: "root" binds here
    if (true) { // 2
              ^~~~~~
  
  Match #3:
  
  /tmp/if-nest.cpp:4:20: note: "root" binds here
    } else if (true) { // 2 or 3?
                     ^~~~~~~~~~~~
  
  Match #4:
  
  /tmp/if-nest.cpp:6:20: note: "root" binds here
    } else if (true) { // 2 or 4?
                     ^~~~~~~~~~~~
  
  Match #5:
  
  /tmp/if-nest.cpp:8:20: note: "root" binds here
    } else if (true) { // 2 or 5?
                     ^~~~~~~~~~~~
  5 matches.


Repository:
  rL LLVM

https://reviews.llvm.org/D32942





More information about the cfe-commits mailing list