[Polly] Revise polly-{update|check}-format targets

[Meinersbur]

Summary

Make clang-format run on each file independently using add_custom_format (instead using a shell script in utils/). The targets polly-{update|check}-format depend on these.

The primary motivation is to make them work on Windows, but also improves them generally:

• Each file update/check can run in parallel (Although they do not take long to run anyway)
• Implicit dependency on clang-format, so it recompiles if necessary
• polly-check-format shows the formatting difference if failing

[grosser]

LGTM.

Looking at this, it would also be great if we would call polly-check-format from check-polly, if clang-format is available in our build (and we are not just a LLVM+Polly build).

Best,
Tobias

[Meinersbur]

In D12837#245090, @grosser wrote:

Looking at this, it would also be great if we would call polly-check-format from check-polly, if clang-format is available in our build (and we are not just a LLVM+Polly build).

In another commit.

I personally think this would be exaggerated. Some white space in the source has no influence on the correctness of build product. Also, LLVM doesn't do this either.

[jdoerfert]

Meinersbur added a comment.

In http://reviews.llvm.org/D12837#245090, @grosser wrote:

Looking at this, it would also be great if we would call polly-check-format from check-polly, if clang-format is available in our build (and we are not just a LLVM+Polly build).

In another commit.

I personally think this would be exaggerated. Some white space in the source has no influence on the correctness of build product. Also, LLVM doesn't do this either.

As long as we break the buildbots with white space errors it makes sense
to break the local unit tests too. I would like such a feature if
possible.

• msg-22803-76.dat219 BDownload

[Meinersbur]

In D12837#245328, @jdoerfert wrote:

Meinersbur added a comment.

In http://reviews.llvm.org/D12837#245090, @grosser wrote:

Looking at this, it would also be great if we would call polly-check-format from check-polly, if clang-format is available in our build (and we are not just a LLVM+Polly build).

In another commit.

I personally think this would be exaggerated. Some white space in the source has no influence on the correctness of build product. Also, LLVM doesn't do this either.

As long as we break the buildbots with white space errors it makes sense
to break the local unit tests too. I would like such a feature if
possible.

You can just run "ninja check-polly polly-check-format".

However, here is the patch: D12850

polly/trunk/CMakeLists.txt

 
 # Add target to check formatting of polly files
 file( GLOB_RECURSE files *.h *.cpp)
 file( GLOB_RECURSE jsonfiles lib/JSON/*.h lib/JSON/*.cpp)
 file( GLOB_RECURSE islfiles lib/External/isl/*.h lib/External/isl/*.c
                    lib/External/isl/include/isl/*.h lib/External/isl/imath/*.h
                    lib/External/isl/imath/*.c)
 list( REMOVE_ITEM files ${jsonfiles} ${islfiles})
-add_custom_command( OUTPUT formatting COMMAND
-  CLANG_FORMAT=${LLVM_BINARY_DIR}/bin/clang-format
-  ${CMAKE_CURRENT_SOURCE_DIR}/utils/check_format.sh ${files})
-add_custom_target(polly-check-format DEPENDS formatting)
+
+set(check_format_depends)
+set(update_format_depends)
+set(i 0)
+foreach (file IN LISTS files)
+  add_custom_command(OUTPUT polly-check-format${i}
+    COMMAND clang-format -style=llvm ${file} | diff -u ${file} -
+    VERBATIM
+    COMMENT "Checking format of ${file}..."
+  )
+  list(APPEND check_format_depends "polly-check-format${i}")
+
+  add_custom_command(OUTPUT polly-update-format${i}
+    COMMAND clang-format -i -style=llvm ${file}
+    VERBATIM
+    COMMENT "Updating format of ${file}..."
+  )
+  list(APPEND update_format_depends "polly-update-format${i}")
+
+  math(EXPR i ${i}+1)
+endforeach ()
+
+add_custom_target(polly-check-format DEPENDS ${check_format_depends})
 set_target_properties(polly-check-format PROPERTIES FOLDER "Polly")
-add_custom_command( OUTPUT formatting-update COMMAND
-  CLANG_FORMAT=${LLVM_BINARY_DIR}/bin/clang-format
-  ${CMAKE_CURRENT_SOURCE_DIR}/utils/update_format.sh ${files})
-add_custom_target(polly-update-format DEPENDS formatting-update)
+
+add_custom_target(polly-update-format DEPENDS ${update_format_depends})
 set_target_properties(polly-update-format PROPERTIES FOLDER "Polly")
 
 # Set the variable POLLY_LINK_LIBS in the llvm/tools/ dir.
 set(POLLY_LINK_LIBS ${POLLY_LINK_LIBS} PARENT_SCOPE)

polly/trunk/utils/check_format.sh

-#!/bin/bash
-
-CLANG_FORMAT=${CLANG_FORMAT}
-
-if [ "${CLANG_FORMAT}x" = "x" ]; then
-  CLANG_FORMAT=`which clang-format`
-  if [ "${CLANG_FORMAT}x" = "x" ]; then
-     echo "Error: cannot find clang-format in your path"
-     exit 1
-  fi
-fi
-
-OK=0
-
-for ARG in "$@"
-  do
-    ${CLANG_FORMAT} -style=llvm $ARG | diff -u $ARG - >&2
-
-    if [[ $? -eq 1 ]]; then
-      OK=1
-    fi
-  done
-
-if [[ $OK -eq "1" ]]; then
-  echo "Error: clang-format reported formatting differences"
-  exit 1
-else
-  echo "OK: clang-format reported no formatting differences"
-  exit 0
-fi
-

polly/trunk/utils/update_format.sh

-#!/bin/bash
-
-CLANG_FORMAT=${CLANG_FORMAT}
-
-if [ "${CLANG_FORMAT}x" = "x" ]; then
-  CLANG_FORMAT=`which clang-format`
-  if [ "${CLANG_FORMAT}x" = "x" ]; then
-     echo "Error: cannot find clang-format in your path"
-     exit 1
-  fi
-fi
-
-for ARG in "$@"
-  do
-    ${CLANG_FORMAT} -i $ARG
-  done