[PATCH] D69000: [clang-tidy] new check: modernize-deprecated-iterator-base

2021-04-09 Thread Nikita Kniazev via Phabricator via cfe-commits
nick updated this revision to Diff 336533. nick edited the summary of this revision. nick added reviewers: njames93, steveire. nick added a comment. Rebased. Now using native `cxxBaseSpecifier` and `hasDirectBase`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-04-03 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment. In D69218#2667523 , @steveire wrote: > What name/email should I use for the commit? Nikita Kniazev CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69218/new/ https://reviews.llvm.org/D69218

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-03-28 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added inline comments. Comment at: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp:301 +TEST_F(RegistryTest, CXXBaseSpecifier) { + // TODO: rewrite with top-level cxxBaseSpecifier matcher when available + DeclarationMatcher ClassHasAnyDirectBase = ste

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-03-28 Thread Nikita Kniazev via Phabricator via cfe-commits
nick updated this revision to Diff 333728. nick added a comment. In D69218#2654614 , @steveire wrote: > @nick Sorry that getting these changes merged takes so long. > > @njames93 If you have an alternative way forward, please let us know what it > is. > >

[PATCH] D99107: [clang][CodeGen] Do not emit NVRO debug helper when not emitting debug info

2021-03-22 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment. Is this hack actually needed? I could not reproduce a problem with https://bugs.chromium.org/p/chromium/issues/detail?id=860398#c13 repro, the breakpoint fires for me and I see the variable. The difference with the hack and without, using `clang.exe src.cpp -S -masm=intel

[PATCH] D99107: [clang][CodeGen] Do not emit NVRO debug helper when not emitting debug info

2021-03-22 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment. In D99107#2642451 , @rnk wrote: > We try to uphold the invariant that -g flags do not affect generated code, so > I don't think we should do this. Even under `-O0`? I could change the check to always emit on `-O0`. With optimizatio

[PATCH] D99107: [clang][CodeGen] Do not emit NVRO debug helper when not emitting debug info

2021-03-22 Thread Nikita Kniazev via Phabricator via cfe-commits
nick created this revision. nick added reviewers: akhuang, rnk, aprantl. nick requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When no debug information is emitted there is no point in emitting a hack introduced in D63361

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-03-21 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3812-3821 AST_POLYMORPHIC_MATCHER_P_OVERLOAD( hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, TypedefNameDecl, -ValueDecl), +

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-03-21 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment. Could you please commit the patch for me? I do not have commit rights. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69218/new/ https://reviews.llvm.org/D69218 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-03-21 Thread Nikita Kniazev via Phabricator via cfe-commits
nick updated this revision to Diff 332151. nick added a comment. Lint fixes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69218/new/ https://reviews.llvm.org/D69218 Files: clang/docs/LibASTMatchersReference.html clang/include/clang/ASTMatchers/ASTMatchers.h clang/lib/ASTMatchers/

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-03-21 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:329 + EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX)); + EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX)); +} aaro

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-03-19 Thread Nikita Kniazev via Phabricator via cfe-commits
nick updated this revision to Diff 331955. nick added a comment. Forgot to remove a duplicated test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69218/new/ https://reviews.llvm.org/D69218 Files: clang/docs/LibASTMatchersReference.html clang/include/clang/ASTMatchers/ASTMatchers.h

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-03-19 Thread Nikita Kniazev via Phabricator via cfe-commits
nick updated this revision to Diff 331945. nick retitled this revision from "[clang][AST] Add `CXXBaseSpecifier` matcher support" to "[ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)". nick edited the summary of this revision. nick added a reviewer: aaron.ballman. nick added a comment.

[PATCH] D82425: [SemaCXX] Fix false positive of -Wuninitialized-const-reference in empty function body.

2020-06-24 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added inline comments. Comment at: clang/lib/Analysis/UninitializedValues.cpp:435 if ((*I)->getType().isConstQualified()) -classify((*I), ConstRefUse); +if (!hasTrivialBody(CE)) + classify((*I), ConstRefUse); zequanwu wrote: >

[PATCH] D82425: [SemaCXX] Fix false positive of -Wuninitialized-const-reference in empty function body.

2020-06-24 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment. Thanks @zequanwu, much appreciated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82425/new/ https://reviews.llvm.org/D82425 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

2020-06-23 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment. > We didn't see it in the code bases I work with, so is boost a special case, > or an example of a common practice? I do not have resources to make such statistics, but there are compilers where casting to void is not enough to suppress the warning. https://herbsutter.com

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

2020-06-23 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment. > I feel like doing interprocedural analysis for this is overkill. What is the > benefit of boost::ignore_unused(foo); rather than the more common (void) > foo;? Any examples? > I haven't seen boost::ignore_unused before. In my experience, the idiomatic > way of ignorin

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

2020-06-22 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment. > This warning can be turned off by the flag > `-Wno-uninitialized-const-reference`. Suggesting to turn off the warning should be the last resort. I am pointing to the false positives for large existing code bases from `-Wall` diagnostic. > I don't think we can just make

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

2020-06-22 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment. This diagnostic bring headaches because frequently `-Wunused-variable` suppression is done via no-op pseudo-consuming function like `boost::ignore_unused` . Particularly, it fires in Boost h

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

2019-10-20 Thread Nikita Kniazev via Phabricator via cfe-commits
nick updated this revision to Diff 225783. nick added a comment. Fixed typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69218/new/ https://reviews.llvm.org/D69218 Files: clang/docs/LibASTMatchersReference.html clang/include/clang/AST/ASTTyp

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

2019-10-19 Thread Nikita Kniazev via Phabricator via cfe-commits
nick updated this revision to Diff 225770. nick added a comment. Regenerated the docs, added more tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69218/new/ https://reviews.llvm.org/D69218 Files: clang/docs/LibASTMatchersReference.html cl

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

2019-10-19 Thread Nikita Kniazev via Phabricator via cfe-commits
nick created this revision. nick added a reviewer: klimek. nick added a project: clang. Herald added a subscriber: cfe-commits. Required for capturing base specifier in matchers: `cxxRecordDecl(hasBase(cxxBaseSpecifier().bind("base")))` Will make implementation of D69000

[PATCH] D69000: [clang-tidy] new check: modernize-deprecated-iterator-base

2019-10-15 Thread Nikita Kniazev via Phabricator via cfe-commits
nick marked 2 inline comments as done. nick added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedIteratorBaseCheck.cpp:218 + // Requires C++. + if (!getLangOpts().CPlusPlus) +return; Eugene.Zelenko wrote: > Should it check fo

[PATCH] D69000: [clang-tidy] new check: modernize-deprecated-iterator-base

2019-10-15 Thread Nikita Kniazev via Phabricator via cfe-commits
nick updated this revision to Diff 225133. nick added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69000/new/ https://reviews.llvm.org/D69000 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt clang-to

[PATCH] D69000: [clang-tidy] new check: modernize-deprecated-iterator-base

2019-10-15 Thread Nikita Kniazev via Phabricator via cfe-commits
nick created this revision. nick added a reviewer: alexfh. nick added a project: clang-tools-extra. Herald added subscribers: cfe-commits, kristof.beyls, xazax.hun, mgorny. Herald added a project: clang. Finds deprecated in C++17 inheritance from `std::iterator` and replaces it with type aliases.