-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang-tidy] Fix modernize-use-nullptr
crash on 32-bit Windows
#160023
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
Conversation
@llvm/pr-subscribers-clang-tools-extra Author: Victor Chernyakin (localspook) ChangesFixes #53778. See that issue for an explanation of the problem. Full diff: https://github.com/llvm/llvm-project/pull/160023.diff 1 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
index c38fb3a01d287..782a10420f9cd 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -53,7 +53,7 @@ StatementMatcher makeCastSequenceMatcher(llvm::ArrayRef<StringRef> NameList) {
unless(hasImplicitDestinationType(
qualType(matchers::matchesAnyListedTypeName(NameList)))));
- auto IsOrHasDescendant = [](auto InnerMatcher) {
+ auto IsOrHasDescendant = [](const auto &InnerMatcher) {
return anyOf(InnerMatcher, hasDescendant(InnerMatcher));
};
|
@llvm/pr-subscribers-clang-tidy Author: Victor Chernyakin (localspook) ChangesFixes #53778. See that issue for an explanation of the problem. Full diff: https://github.com/llvm/llvm-project/pull/160023.diff 1 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
index c38fb3a01d287..782a10420f9cd 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -53,7 +53,7 @@ StatementMatcher makeCastSequenceMatcher(llvm::ArrayRef<StringRef> NameList) {
unless(hasImplicitDestinationType(
qualType(matchers::matchesAnyListedTypeName(NameList)))));
- auto IsOrHasDescendant = [](auto InnerMatcher) {
+ auto IsOrHasDescendant = [](const auto &InnerMatcher) {
return anyOf(InnerMatcher, hasDescendant(InnerMatcher));
};
|
Will be good idea to mention fix in Release Notes. |
I keep forgetting to do that >_< |
Co-authored-by: EugeneZelenko <[email protected]>
Awesome, thank you for looking into this! Do we have the possibility to verify this before merging? I read your analysis and I think it makes sense, but would of course be good to test that the fix does work. |
Sorry if I wasn't clear; I have a local x86 build of clang-tidy, I've verified that all the alternative fixes I've listed in the issue do work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, wish we could automatically detect that because this is very easy to miss. Could we maybe remove &
from tuple to always store values.
cc @AaronBallman as maintainer of AST matchers |
Should retry pinging after 6th of october (preferably give one more week), https://discourse.llvm.org/t/my-upcoming-availability/87851 |
Ooh, thank you, I wasn’t aware of that post. Since it says another maintainer can step in in the meantime, cc @Sirraide as maintainer of AST visitors, which seems like the next closest component |
AST matchers are something I’m very much not familiar w/ unfortunately (yes, there’s an AST visitor in there somewhere but it’s more than just that ;Þ) |
Co-authored-by: EugeneZelenko <[email protected]>
Ah, no worries then! |
|
I think we can merge this now and later take steps on how to avoid similar issues in the future. |
Fair enough, merging |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/15398 Here is the relevant piece of the build log for the reference
|
Failure looks unrelated |
Fixes #53778. See that issue for an explanation of the problem.