Skip to content

Commit

Permalink
presubmits: Add a presubmit for CHECK and related macros.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pizzud authored and copybara-github committed Apr 4, 2024
1 parent c2cc020 commit 3d6326c
Showing 1 changed file with 16 additions and 0 deletions.
16 changes: 16 additions & 0 deletions .github/workflows/check_contents.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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[<op>] 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
Expand Down

0 comments on commit 3d6326c

Please sign in to comment.