This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Use a python script instead of a shell script to run clang-tidy tests.
ClosedPublic

Authored by alexfh on Aug 19 2015, 6:07 PM.

Details

Summary

Add check_clang_tidy.py script that is functionally identical to the
check_clang_tidy.py, but should also be functional on windows.

I've verified that the script works on linux. Would be nice if folks using
Windows could test the patch before I break windows bots ;)

Diff Detail

Repository
rL LLVM

Event Timeline

alexfh updated this revision to Diff 32644.Aug 19 2015, 6:07 PM
alexfh retitled this revision from to [clang-tidy] Use a python script instead of a shell script to run clang-tidy tests..
alexfh updated this object.
alexfh added reviewers: aaron.ballman, chapuni.
alexfh added a subscriber: cfe-commits.
alexfh updated this revision to Diff 32645.Aug 19 2015, 6:10 PM

Removed an unused import.

aaron.ballman edited edge metadata.Aug 20 2015, 7:17 AM

Yay! I'm really excited for this! Unfortunately, it fails all over the place on Windows. The errors are all in the form:

58> ****
58> FAIL: Clang Tools :: clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp (7280 of 23187)
58> **** TEST 'Clang Tools :: clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp' FAILED ****
58> Script:
58> --
58> $(dirname E:\llvm\llvm\tools\clang\tools\extra\test\clang-tidy\readability-simplify-bool-expr-chained-conditional-return.cpp)/check_clang_tidy.py E:\llvm\llvm\tools\clang\tools\extra\test\clang-tidy\readability-simplify-bool-expr-chained-conditional-return.cpp readability-simplify-boolean-expr E:\llvm\2015\tools\clang\tools\extra\test\clang-tidy\Output\readability-simplify-bool-expr-chained-conditional-return.cpp.tmp -config="{CheckOptions: [:+"readability-simplify-boolean-expr.ChainedConditionalReturn",+value:+1]}" --
58> --
58> Exit Code: 127
58>
58> Command Output (stdout):
58> --
58> Command 0: "$(dirname" "E:\llvm\llvm\tools\clang\tools\extra\test\clang-tidy\readability-simplify-bool-expr-chained-conditional-return.cpp)/check_clang_tidy.py" "E:\llvm\llvm\tools\clang\tools\extra\test\clang-tidy\readability-simplify-bool-expr-chained-conditional-return.cpp" "readability-simplify-boolean-expr" "E:\llvm\2015\tools\clang\tools\extra\test\clang-tidy\Output\readability-simplify-bool-expr-chained-conditional-return.cpp.tmp" "-config={CheckOptions: [:+readability-simplify-boolean-expr.ChainedConditionalReturn,+value:+1]}" "--"
58> Command 0 Result: 127
58> Command 0 Output:
58>
58>
58> Command 0 Stderr:
58> '$(dirname': command not found
58>
58>
58> --
58>
58> ****
58> FAIL: Clang Tools :: clang-tidy/readability-identifier-naming.cpp (7281 of 23187)
58> **** TEST 'Clang Tools :: clang-tidy/readability-identifier-naming.cpp' FAILED ****
58> Script:
58> --
58> $(dirname E:\llvm\llvm\tools\clang\tools\extra\test\clang-tidy\readability-identifier-naming.cpp)/check_clang_tidy.py E:\llvm\llvm\tools\clang\tools\extra\test\clang-tidy\readability-identifier-naming.cpp readability-identifier-naming E:\llvm\2015\tools\clang\tools\extra\test\clang-tidy\Output\readability-identifier-naming.cpp.tmp -config='{CheckOptions: [ :+readability-identifier-naming.AbstractClassCase,+value:+CamelCase, :+readability-identifier-naming.AbstractClassPrefix,+value:+'A', :+readability-identifier-naming.ClassCase,+value:+CamelCase, :+readability-identifier-naming.ClassPrefix,+value:+'C', :+readability-identifier-naming.ClassConstantCase,+value:+CamelCase, :+readability-identifier-naming.ClassConstantPrefix,+value:+'k', :+readability-identifier-naming.ClassMemberCase,+value:+CamelCase, :+readability-identifier-naming.ClassMethodCase,+value:+camelBack, :+readability-identifier-naming.ConstantCase,+value:+UPPER_CASE, :+readability-identifier-naming.ConstantSuffix,+value:+'_CST', :+readability-identifier-naming.ConstexprFunctionCase,+value:+lower_case, :+readability-identifier-naming.ConstexprMethodCase,+value:+lower_case, :+readability-identifier-naming.ConstexprVariableCase,+value:+lower_case, :+readability-identifier-naming.EnumCase,+value:+CamelCase, :+readability-identifier-naming.EnumPrefix,+value:+'E', :+readability-identifier-naming.EnumConstantCase,+value:+UPPER_CASE, :+readability-identifier-naming.FunctionCase,+value:+camelBack, :+readability-identifier-naming.GlobalConstantCase,+value:+UPPER_CASE, :+readability-identifier-naming.GlobalFunctionCase,+value:+CamelCase, :+readability-identifier-naming.GlobalVariableCase,+value:+lower_case, :+readability-identifier-naming.GlobalVariablePrefix,+value:+'g_', :+readability-identifier-naming.InlineNamespaceCase,+value:+lower_case, :+readability-identifier-naming.LocalConstantCase,+value:+CamelCase, :+readability-identifier-naming.LocalConstantPrefix,+value:+'k', :+readability-identifier-naming.LocalVariableCase,+value:+lower_case, :+readability-identifier-naming.MemberCase,+value:+CamelCase, :+readability-identifier-naming.MemberPrefix,+value:+'m_', :+readability-identifier-naming.ConstantMemberCase,+value:+lower_case, :+readability-identifier-naming.PrivateMemberPrefix,+value:+'__', :+readability-identifier-naming.ProtectedMemberPrefix,+value:+'_', :+readability-identifier-naming.PublicMemberCase,+value:+lower_case, :+readability-identifier-naming.MethodCase,+value:+camelBack, :+readability-identifier-naming.PrivateMethodPrefix,+value:+'__', :+readability-identifier-naming.ProtectedMethodPrefix,+value:+'_', :+readability-identifier-naming.NamespaceCase,+value:+lower_case, :+readability-identifier-naming.ParameterCase,+value:+camelBack, :+readability-identifier-naming.ParameterPrefix,+value:+'a_', :+readability-identifier-naming.ConstantParameterCase,+value:+camelBack, :+readability-identifier-naming.ConstantParameterPrefix,+value:+'i_', :+readability-identifier-naming.ParameterPackCase,+value:+camelBack, :+readability-identifier-naming.PureFunctionCase,+value:+lower_case, :+readability-identifier-naming.PureMethodCase,+value:+camelBack, :+readability-identifier-naming.StaticConstantCase,+value:+UPPER_CASE, :+readability-identifier-naming.StaticVariableCase,+value:+camelBack, :+readability-identifier-naming.StaticVariablePrefix,+value:+'s_', :+readability-identifier-naming.StructCase,+value:+lower_case, :+readability-identifier-naming.TemplateParameterCase,+value:+UPPER_CASE, :+readability-identifier-naming.TemplateTemplateParameterCase,+value:+CamelCase, :+readability-identifier-naming.TemplateUsingCase,+value:+lower_case, :+readability-identifier-naming.TemplateUsingPrefix,+value:+'u_', :+readability-identifier-naming.TypeTemplateParameterCase,+value:+camelBack, :+readability-identifier-naming.TypeTemplateParameterSuffix,+value:+'_t', :+readability-identifier-naming.TypedefCase,+value:+lower_case, :+readability-identifier-naming.TypedefSuffix,+value:+'_t', :+readability-identifier-naming.UnionCase,+value:+CamelCase, :+readability-identifier-naming.UnionPrefix,+value:+'U', :+readability-identifier-naming.UsingCase,+value:+lower_case, :+readability-identifier-naming.ValueTemplateParameterCase,+value:+camelBack, :+readability-identifier-naming.VariableCase,+value:+lower_case, :+readability-identifier-naming.VirtualMethodCase,+value:+UPPER_CASE, :+readability-identifier-naming.VirtualMethodPrefix,+value:+'v_', :+readability-identifier-naming.IgnoreFailedSplit,+value:+0 ]}' -- -std=c++11 -fno-delayed-template-parsing
58> --
58> Exit Code: 127
58>
58> Command Output (stdout):
58> --
58> Command 0: "$(dirname" "E:\llvm\llvm\tools\clang\tools\extra\test\clang-tidy\readability-identifier-naming.cpp)/check_clang_tidy.py" "E:\llvm\llvm\tools\clang\tools\extra\test\clang-tidy\readability-identifier-naming.cpp" "readability-identifier-naming" "E:\llvm\2015\tools\clang\tools\extra\test\clang-tidy\Output\readability-identifier-naming.cpp.tmp" "-config={CheckOptions: [ :+readability-identifier-naming.AbstractClassCase,+value:+CamelCase, :+readability-identifier-naming.AbstractClassPrefix,+value:+A, :+readability-identifier-naming.ClassCase,+value:+CamelCase, :+readability-identifier-naming.ClassPrefix,+value:+C, :+readability-identifier-naming.ClassConstantCase,+value:+CamelCase, :+readability-identifier-naming.ClassConstantPrefix,+value:+k, :+readability-identifier-naming.ClassMemberCase,+value:+CamelCase, :+readability-identifier-naming.ClassMethodCase,+value:+camelBack, :+readability-identifier-naming.ConstantCase,+value:+UPPER_CASE, :+readability-identifier-naming.ConstantSuffix,+value:+_CST, :+readability-identifier-naming.ConstexprFunctionCase,+value:+lower_case, :+readability-identifier-naming.ConstexprMethodCase,+value:+lower_case, :+readability-identifier-naming.ConstexprVariableCase,+value:+lower_case, :+readability-identifier-naming.EnumCase,+value:+CamelCase, :+readability-identifier-naming.EnumPrefix,+value:+E, :+readability-identifier-naming.EnumConstantCase,+value:+UPPER_CASE, :+readability-identifier-naming.FunctionCase,+value:+camelBack, :+readability-identifier-naming.GlobalConstantCase,+value:+UPPER_CASE, :+readability-identifier-naming.GlobalFunctionCase,+value:+CamelCase, :+readability-identifier-naming.GlobalVariableCase,+value:+lower_case, :+readability-identifier-naming.GlobalVariablePrefix,+value:+g_, :+readability-identifier-naming.InlineNamespaceCase,+value:+lower_case, :+readability-identifier-naming.LocalConstantCase,+value:+CamelCase, :+readability-identifier-naming.LocalConstantPrefix,+value:+k, :+readability-identifier-naming.LocalVariableCase,+value:+lower_case, :+readability-identifier-naming.MemberCase,+value:+CamelCase, :+readability-identifier-naming.MemberPrefix,+value:+m_, :+readability-identifier-naming.ConstantMemberCase,+value:+lower_case, :+readability-identifier-naming.PrivateMemberPrefix,+value:+__, :+readability-identifier-naming.ProtectedMemberPrefix,+value:+_, :+readability-identifier-naming.PublicMemberCase,+value:+lower_case, :+readability-identifier-naming.MethodCase,+value:+camelBack, :+readability-identifier-naming.PrivateMethodPrefix,+value:+__, :+readability-identifier-naming.ProtectedMethodPrefix,+value:+_, :+readability-identifier-naming.NamespaceCase,+value:+lower_case, :+readability-identifier-naming.ParameterCase,+value:+camelBack, :+readability-identifier-naming.ParameterPrefix,+value:+a_, :+readability-identifier-naming.ConstantParameterCase,+value:+camelBack, :+readability-identifier-naming.ConstantParameterPrefix,+value:+i_, :+readability-identifier-naming.ParameterPackCase,+value:+camelBack, :+readability-identifier-naming.PureFunctionCase,+value:+lower_case, :+readability-identifier-naming.PureMethodCase,+value:+camelBack, :+readability-identifier-naming.StaticConstantCase,+value:+UPPER_CASE, :+readability-identifier-naming.StaticVariableCase,+value:+camelBack, :+readability-identifier-naming.StaticVariablePrefix,+value:+s_, :+readability-identifier-naming.StructCase,+value:+lower_case, :+readability-identifier-naming.TemplateParameterCase,+value:+UPPER_CASE, :+readability-identifier-naming.TemplateTemplateParameterCase,+value:+CamelCase, :+readability-identifier-naming.TemplateUsingCase,+value:+lower_case, :+readability-identifier-naming.TemplateUsingPrefix,+value:+u_, :+readability-identifier-naming.TypeTemplateParameterCase,+value:+camelBack, :+readability-identifier-naming.TypeTemplateParameterSuffix,+value:+_t, :+readability-identifier-naming.TypedefCase,+value:+lower_case, :+readability-identifier-naming.TypedefSuffix,+value:+_t, :+readability-identifier-naming.UnionCase,+value:+CamelCase, :+readability-identifier-naming.UnionPrefix,+value:+U, :+readability-identifier-naming.UsingCase,+value:+lower_case, :+readability-identifier-naming.ValueTemplateParameterCase,+value:+camelBack, :+readability-identifier-naming.VariableCase,+value:+lower_case, :+readability-identifier-naming.VirtualMethodCase,+value:+UPPER_CASE, :+readability-identifier-naming.VirtualMethodPrefix,+value:+v_, :+readability-identifier-naming.IgnoreFailedSplit,+value:+0 ]}" "--" "-std=c++11" "-fno-delayed-template-parsing"
58> Command 0 Result: 127
58> Command 0 Output:
58>
58>
58> Command 0 Stderr:
58> '$(dirname': command not found
58>
58>
58> --
58>
58> ****

Is dirname something lit is supposed to be handling for us, or is that a shell command that's not supported on Windows?

~Aaron

chapuni edited edge metadata.Aug 20 2015, 7:34 AM

Alex, thanks for your working. I tweaked to run on my hosts.

  • mingw-w64 with Py3.3
  • Linux with Py2.7 and Py3.4

My change is here; https://github.com/chapuni/llvm-project/commit/0f4a303bdc7b925f42c0cd48817052d2b49b2234
Could you merge it, please?

test/clang-tidy/arg-comments.cpp
1 ↗(On Diff #32645)

$(dirname %s) assumes shell. use %S instead.
Lit internal runner is not capable of !shbang. use %python.

// RUN: %python %S/check_clang_tidy.py %s misc-argument-comment %t
test/clang-tidy/check_clang_tidy.py
71 ↗(On Diff #32645)

It is not string, but bytes. It wouldn't run on py3.

+  clang_tidy_output = subprocess.check_output(args, stderr=subprocess.STDOUT).decode()
85 ↗(On Diff #32645)

It cannot be concatenated to string. Use encode.

+        diff_output.decode() +
test/clang-tidy/google-readability-casting.c
7 ↗(On Diff #32645)

It can be pruned.

test/clang-tidy/misc-unused-parameters.cpp
5 ↗(On Diff #32645)

It can be pruned.

alexfh updated this revision to Diff 32705.Aug 20 2015, 9:48 AM
alexfh marked 4 inline comments as done.
alexfh edited edge metadata.

Addressed review comments.

alexfh added inline comments.Aug 20 2015, 9:50 AM
test/clang-tidy/arg-comments.cpp
1 ↗(On Diff #32645)

Sure, I should have noticed this. Thanks for pointing out!

test/clang-tidy/google-readability-casting.c
7 ↗(On Diff #32645)

Line 5 runs cp, will it work on Windows?

chapuni accepted this revision.Aug 20 2015, 10:14 AM
chapuni edited edge metadata.

Ready to commit regardless of one REQUIRES.

Testing Time: 4.73s
  Expected Passes    : 72

Note, llvm-include-order.cpp will XFAIL for targeting msvc.

test/clang-tidy/google-readability-casting.c
7 ↗(On Diff #32705)

It should work.
We require GnuWin32 to run tests on Windows. It has cp(1).

This revision is now accepted and ready to land.Aug 20 2015, 10:14 AM
aaron.ballman accepted this revision.Aug 20 2015, 10:24 AM
aaron.ballman edited edge metadata.

The latest patch works for me when testing on Windows with MSVC. LGTM, and thank you!

alexfh closed this revision.Aug 20 2015, 10:59 AM
This revision was automatically updated to reflect the committed changes.
alexfh added inline comments.Aug 20 2015, 11:00 AM
test/clang-tidy/google-readability-casting.c
7 ↗(On Diff #32705)

Removed REQUIRES: shell.