[update_cc_test_checks] Don't attach CHECK lines to function declarations

[arichardson]

Summary

Previously we were adding the CHECK lines to both definitions and
declarations. Update the JSON AST dump parsing code to skip all
FunctionDecls without an "inner" node (i.e. no body).

[arichardson]

Remove debug output

[merge_guards_bot]

Unit tests: fail. 62344 tests passed, 1 failed and 839 were skipped.

failed: libc++.std/containers/sequences/array/array_creation/to_array.fail.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[MaskRay]

llvm/test/tools/UpdateTestChecks/update_cc_test_checks/def-and-decl.test

4

-f is not needed.

5

diff -u -> cmp

llvm/utils/update_cc_test_checks.py

80

Does 'inner' in node work?

[arichardson]

llvm/test/tools/UpdateTestChecks/update_cc_test_checks/def-and-decl.test

5

I prefer diff -u so that you get useful output on failure. It's using the lit-internal diff so it's portable.

llvm/utils/update_cc_test_checks.py

80

Yes, changed it. Thanks.

[arichardson]

Address feedback

[merge_guards_bot]

Unit tests: pass. 62362 tests passed, 0 failed and 839 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[greened]

LGTM. I did something similar in my local copy that I was going to upstream but I added a switch for the user to choose where to place checks (declaration, definition or both). I only added the switch because I wasn't sure whether the changed behavior was intended. I'm fine just placing checks on definitions as I don't have a need to put them on declarations.

llvm/test/tools/UpdateTestChecks/update_cc_test_checks/Inputs/def-and-decl.c

+// Check that the CHECK lines are generated before the definition and not the declaration
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -emit-llvm -o - | FileCheck %s
+
+int foo();
+
+void empty_function();
+
+int main() {
+  empty_function();
+  return foo();
+}
+
+int foo() {
+  return 1;
+}
+
+void empty_function() {}

llvm/test/tools/UpdateTestChecks/update_cc_test_checks/Inputs/def-and-decl.c.expected

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// Check that the CHECK lines are generated before the definition and not the declaration
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -emit-llvm -o - | FileCheck %s
+
+int foo();
+
+void empty_function();
+
+// CHECK-LABEL: @main(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[RETVAL:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    store i32 0, i32* [[RETVAL]], align 4
+// CHECK-NEXT:    call void @empty_function()
+// CHECK-NEXT:    [[CALL:%.*]] = call i32 @foo()
+// CHECK-NEXT:    ret i32 [[CALL]]
+//
+int main() {
+  empty_function();
+  return foo();
+}
+
+// CHECK-LABEL: @foo(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    ret i32 1
+//
+int foo() {
+  return 1;
+}
+
+// CHECK-LABEL: @empty_function(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    ret void
+//
+void empty_function() {}

llvm/test/tools/UpdateTestChecks/update_cc_test_checks/def-and-decl.test

+## Test that CHECK lines are generated before the definion and not the declaration
+
+# RUN: cp %S/Inputs/def-and-decl.c %t.c && %update_cc_test_checks %t.c
+# RUN: diff -u %S/Inputs/def-and-decl.c.expected %t.c
+## Check that re-running update_cc_test_checks doesn't change the output
+# RUN: %update_cc_test_checks %t.c
+# RUN: diff -u %S/Inputs/def-and-decl.c.expected %t.c

llvm/utils/update_cc_test_checks.py

if node.get('isImplicit') is True and node.get('storageClass') == 'extern':
       common.debug('Skipping builtin function:', node['name'], '@', node['loc'])
       return
     common.debug('Found function:', node['kind'], node['name'], '@', node['loc'])
     line = node['loc'].get('line')
     # If there is no line it is probably a builtin function -> skip
     if line is None:
       common.debug('Skipping function without line number:', node['name'], '@', node['loc'])
       return
+    # If there is no 'inner' object, it is a function declaration -> skip
+    if 'inner' not in node:
+      common.debug('Skipping function without body:', node['name'], '@', node['loc'])
+      return
     spell = node['name']
     mangled = node.get('mangledName', spell)
     ret[int(line)-1] = (spell, mangled)
 
   ast = json.loads(stdout)
   if ast['kind'] != 'TranslationUnitDecl':
     common.error('Clang AST dump JSON format changed?')
     sys.exit(2)