Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lint] Add check to prevent usage of #include <regex> #16412

Merged
merged 1 commit into from
Mar 10, 2024

Conversation

Lunderberg
Copy link
Contributor

Currently, the pytorch wheels available through pip install use the pre-C++11 ABI by setting -DUSE_CXX11_ABI=0 [0]. If TVM were to user the pre-C++11 ABI, this would cause breakages with dynamically-linked LLVM environments.

This commit adds a lint check to search for use of #include <regex> in any C++ files. Use of this header should be avoided, as its implementation is not supported by gcc's dual ABI. This ABI incompatibility results in runtime errors either when std::regex is called from TVM, or when std::regex is called from pytorch, depending on which library was loaded first.

This restriction can be removed when a version of pytorch compiled using -DUSE_CXX11_ABI=1 is available from PyPI.

[0] pytorch/pytorch#51039

@Lunderberg
Copy link
Contributor Author

This rule is currently implemented in terms of find and grep, as I couldn't find any options in cpplint to check for specific header files, and clang-tidy would be too much overhead for a single linting rule.

@Lunderberg
Copy link
Contributor Author

The PR #16465 exposes a PackedFunc wrapper for Python's regex matching. The limited functionality it has, checking for a match without returning any extracted results, is sufficient for all existing uses of #include <regex> found by the new lint rule. After it lands, this PR can be updated to use the new wrapper function instead of std::regex.

@Lunderberg Lunderberg force-pushed the lint_forbid_std_regex branch 2 times, most recently from ed1ac32 to 67be0c1 Compare February 8, 2024 13:32
@Lunderberg
Copy link
Contributor Author

Merged from main to PR branch, to resolve CI breakage that was fixed in #16546.

@Lunderberg
Copy link
Contributor Author

Rebased this PR onto main, in order to update the stale CI results.

Currently, the pytorch wheels available through `pip install` use the
pre-C++11 ABI by setting `-DUSE_CXX11_ABI=0` [0].  If TVM were to user
the pre-C++11 ABI, this would cause breakages with dynamically-linked
LLVM environments.

This commit adds a lint check to search for use of `#include <regex>`
in any C++ files.  Use of this header should be avoided, as its
implementation is not supported by gcc's dual ABI.  This ABI
incompatibility results in runtime errors either when `std::regex` is
called from TVM, or when `std::regex` is called from pytorch,
depending on which library was loaded first.

This restriction can be removed when a version of pytorch compiled
using `-DUSE_CXX11_ABI=1` is available from PyPI.

[0] pytorch/pytorch#51039
@tqchen tqchen merged commit 7ab970d into apache:main Mar 10, 2024
18 checks passed
@Lunderberg Lunderberg deleted the lint_forbid_std_regex branch March 10, 2024 17:20
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Mar 12, 2024
Currently, the pytorch wheels available through `pip install` use the
pre-C++11 ABI by setting `-DUSE_CXX11_ABI=0` [0].  If TVM were to user
the pre-C++11 ABI, this would cause breakages with dynamically-linked
LLVM environments.

This commit adds a lint check to search for use of `#include <regex>`
in any C++ files.  Use of this header should be avoided, as its
implementation is not supported by gcc's dual ABI.  This ABI
incompatibility results in runtime errors either when `std::regex` is
called from TVM, or when `std::regex` is called from pytorch,
depending on which library was loaded first.

This restriction can be removed when a version of pytorch compiled
using `-DUSE_CXX11_ABI=1` is available from PyPI.

[0] pytorch/pytorch#51039
thaisacs pushed a commit to thaisacs/tvm that referenced this pull request Apr 3, 2024
Currently, the pytorch wheels available through `pip install` use the
pre-C++11 ABI by setting `-DUSE_CXX11_ABI=0` [0].  If TVM were to user
the pre-C++11 ABI, this would cause breakages with dynamically-linked
LLVM environments.

This commit adds a lint check to search for use of `#include <regex>`
in any C++ files.  Use of this header should be avoided, as its
implementation is not supported by gcc's dual ABI.  This ABI
incompatibility results in runtime errors either when `std::regex` is
called from TVM, or when `std::regex` is called from pytorch,
depending on which library was loaded first.

This restriction can be removed when a version of pytorch compiled
using `-DUSE_CXX11_ABI=1` is available from PyPI.

[0] pytorch/pytorch#51039
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants