[PATCH] D46027: [clang-tidy] Fix PR35824

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5c5e860535d8: [clang-tidy] Fix PR35824 (authored by xazax.hun). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D46027?vs=231078&id=231301#toc Repository: rG LLVM Git

[PATCH] D46027: [clang-tidy] Fix PR35824

2019-11-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. > This approach will also introduce false negatives. Could you add a test showing this with a FIXME comment? > A better approach would be to check if the null statement is the result of > folding an if constexpr. > The current AST API does

[PATCH] D46027: [clang-tidy] Fix PR35824

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Thanks! Updated context for this patch: A superior fix would be to follow through with the approach suggested by Richard in https://reviews.llvm.org/D46234 . However, since I do not have time to finish that and there are people complaining in the PR, I think it is be

[PATCH] D46027: [clang-tidy] Fix PR35824

2019-11-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. Maybe a short notice in the release notes wouldn't hurt. Otherwise LGTM *EDIT*: Aaron commented as well. Plz let him react before committing :) CHANGES SINCE LAST ACTION https://revie

[PATCH] D46027: [clang-tidy] Fix PR35824

2019-11-26 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 231078. xazax.hun added a comment. Herald added subscribers: Charusso, gamesh411. - Use matcher. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46027/new/ https://reviews.llvm.org/D46027 Files: clang-tools-extra/clang-tidy/bugprone/SuspiciousSem

[PATCH] D46027: [clang-tidy] Fix PR35824

2018-10-10 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In https://reviews.llvm.org/D46027#1259989, @ZaMaZaN4iK wrote: > What is the status of the PR? I believe xazax doesnt have time to work further, you can commandeer if you want :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46027 _

[PATCH] D46027: [clang-tidy] Fix PR35824

2018-10-09 Thread Alexander Zaitsev via Phabricator via cfe-commits
ZaMaZaN4iK added a comment. Herald added a subscriber: Szelethus. What is the status of the PR? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D46027: [clang-tidy] Fix PR35824

2018-04-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. > Which solution do you prefer? If I understand the issue properly: both. :-) Having the AST track information that's been folded away is still useful -- some users are using the AST for purposes other than codegen, and the fact that a construct has been folded a

[PATCH] D46027: [clang-tidy] Fix PR35824

2018-04-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added a reviewer: alexfh. Herald added subscribers: dkrupp, rnkovacs, baloghadamsoftware, whisperity. This approach will also introduce false negatives. A better approach would be to check if the null statement is the result of folding an `if constexpr`.