From 3d6326c3213f4b2d48903119c250ffb5458b2e07 Mon Sep 17 00:00:00 2001 From: pizzud Date: Thu, 4 Apr 2024 13:25:52 -0700 Subject: [PATCH] presubmits: Add a presubmit for CHECK and related macros. Generally it's a better idea to find a way to report the error rather than to crash the process. When XLA is used in a server, crashes expose the server to potential queries of death when unexpected data or API calls occur. Removing existing crashes is a longer-term fixit-type effort, but at least we can prevent new intentional crashes from slipping in. PiperOrigin-RevId: 621952801 --- .github/workflows/check_contents.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/workflows/check_contents.yml b/.github/workflows/check_contents.yml index abbf80a18db63..ba9a2ca8a9edf 100644 --- a/.github/workflows/check_contents.yml +++ b/.github/workflows/check_contents.yml @@ -111,6 +111,22 @@ jobs: --suppression_regex '(?i)non-absl ok' --failure_message 'Use Abseil types and functions instead of those in Tensorflow/TSL.'" + - name: "Avoid CHECK and other crashing macros in non-test C++" + if: always() # We want to run all checks, not just til first failure + run: "python3 build_tools/lint/check_contents.py" + # We want to match TF_[Q]CHECK_OK, as well as the ABSL [Q]CHECK[] and both + # ABSL and TF [Q]FATAL logging. The CHECK macros need to look for an open parenthesis + # to make sure that we don't accidentally match lit expressions like // CHECK:. + run: "python3 build_tools/lint/check_contents.py + --path_regex_exclusion '(?:.github/workflows/check_contents\\.yml|.*_test.cc)' + --path_regex '.*\.(cpp|cc|h)$' + --prohibited_regex '\\b(\ + (?:TF_)?Q?CHECK(?:_OK|_EQ|_NE|_GT|_GE|_LT|_LE)?\\(\ + (?:ABSL)|TF_)?LOG\\(?FATAL\\)\ + )' + --suppression_regex '(?i)crash ok' + --failure_message 'Avoid crashing in favor of returning a Status or other error. If you\ + must crash, add a // crash ok annotation to the line.'" # ----------------Python checks---------------- - name: "Check for python print()" if: always() # We want to run all checks, not just til first failure