[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

2019-05-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: alexfh. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D61480 Files: clang-tools-extra/clang-tidy/bugprone/InaccurateEraseCheck.cpp clang/includ

[PATCH] D61485: Added an assert in `isConstantInitializer`: initializer lists must be in semantic form

2019-05-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D61485 Files: clang/lib/AST/Expr.cpp Index: clang/lib/AST/Expr.cpp ==

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

2019-05-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr marked 2 inline comments as done. gribozavr added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:2079 +} + TEST(EqualsBoundNodeMatcher, QualType) { aaron.ballman wrote: > Can you also add a negative test case for:

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

2019-05-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 197950. gribozavr added a comment. Regenerated documentation and added one more test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61480/new/ https://reviews.llvm.org/D61480 Files: clang-tools-extra/clang-

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

2019-05-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. gribozavr marked an inline comment as done. Closed by commit rCTE359876: Added an AST matcher for declarations that are in the `std` namespace (authored by gribozavr, committed by ). Changed prior to commit: https://revie

[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Constant evaluator does not work on value-dependent or type-dependent expressions. Also fixed bugs uncovered by these assertions. Repository: rG LLVM Github Monorepo https://reviews.llvm.o

[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. > Can you add tests for the bugs you fixed? thanks The bugs were detected by existing tests (those tests triggered the newly added assertions). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61522/new/ https://reviews.ll

[PATCH] D58573: [analyzer] Move UninitializedObjectChecker out of alpha

2019-05-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. Thank you for the fixes! Another crash: https://bugs.llvm.org/show_bug.cgi?id=41741 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58573/new/ https://reviews.llvm.org/D58573 ___ cfe-commit

[PATCH] D61644: Documentation for bugprone-inaccurate-erase: added an example of a bug that this checker catches

2019-05-07 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: alexfh. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D61644 Files: clang-tools-extra/docs/clang-tidy/checks/bugprone-inaccurate-erase.rst Inde

[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-07 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:5784 Expr::EvalResult Result; -if (CollapseLoopCountExpr->EvaluateAsInt(Result, SemaRef.getASTContext())) +if (!CollapseLoopCountExpr->isValueDepend

[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

2019-05-07 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp:55 + +// Return the string for the Objective-C message receiver. Keeps any generics +// in

[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 198603. gribozavr added a comment. Addressed review comments: - Simplified error handling in SemaOpenMP, - Only check isValueDependent. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61522/new/ https://revie

[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr marked an inline comment as done. gribozavr added a comment. In D61522#1494155 , @rsmith wrote: > The right thing to check in all of these cases should be only > `isValueDependent()`. Every type-dependent expression should generally also > be v

[PATCH] D61485: Added an assert in `isConstantInitializer`: initializer lists must be in semantic form

2019-05-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. @rsmith Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61485/new/ https://reviews.llvm.org/D61485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D61644: Documentation for bugprone-inaccurate-erase: added an example of a bug that this checker catches

2019-05-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE360247: Documentation for bugprone-inaccurate-erase: added an example of a bug that… (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D61644?vs=198492&id=19862

[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:6369 // very difficult. Ideally, we should handle them more gracefully. -if (!EIA->getCond()->EvaluateWithSubstitution( +if (EIA->getCond()->isVal

[PATCH] D61739: check_clang_tidy.py now passes `-format-style=none` to clang_tidy

2019-05-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: alexfh. Herald added a project: clang. Herald added a subscriber: cfe-commits. If the test does not specify a formatting style, force "none"; otherwise autodetection logic can discover a ".clang-tidy" file that is not related to the test

[PATCH] D61739: check_clang_tidy.py now passes `-format-style=none` to clang_tidy

2019-05-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL360358: check_clang_tidy.py now passes `-format-style=none` to clang_tidy (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to co

[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

2019-02-21 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. macOS supports `pthread_setschedparam()` that we call on Linux, why not reuse the same code path? https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/pthread_setschedparam.3.html Repository: rCTE Clang Tools Extra CHA

[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

2019-02-22 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. I see, there's no SCHED_IDLE in the macOS SDK. OK then, I trust that people who wrote CIndex.cpp support for macOS probably have got it correctly working for macOS, so it makes sense to

[PATCH] D58601: Fixed grammar in index.rst

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a subscriber: arphaman. Herald added a project: clang. gribozavr added a reviewer: ilya-biryukov. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58601 Files: clang-tools-extra/docs/index.rst Index: clang-tools-extra/docs/index.rs

[PATCH] D58602: Removed an unhelpful comment in index.rst

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: ilya-biryukov. Herald added subscribers: jdoerfert, arphaman. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58602 Files: clang-tools-extra/docs/index.rst Index: clang-tools-extra/doc

[PATCH] D58603: Updated the documentation build instructions for the current CMake build system

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: ilya-biryukov. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58603 Files: clang-tools-extra/docs/README.txt Index: clang-tools-extra/docs/README.txt =

[PATCH] D58607: Moved clangd docs to a separate directory in preparation to restructure them into multiple files

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: ilya-biryukov. Herald added subscribers: jdoerfert, kadircet, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58607 Files: clang-tools-extra/docs/cla

[PATCH] D58607: Moved clangd docs to a separate directory in preparation to restructure them into multiple files

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang-tools-extra/docs/clangd.rst:1 - -Clangd - +:orphan: ilya-biryukov wrote: > Having this does page not seem useful in the long-run. > Do we plan

[PATCH] D58603: Updated the documentation build instructions for the current CMake build system

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 188133. gribozavr marked 2 inline comments as done. gribozavr added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58603/new/ https://reviews.llvm.org/D58603 Files: clang-tools-extra/docs/READM

[PATCH] D58602: Removed an unhelpful comment in index.rst

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL354777: Removed an unhelpful comment in index.rst (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.

[PATCH] D58601: Fixed grammar in index.rst

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE354778: Fixed grammar in index.rst (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D58601?vs=188117&id=188135#toc Repository: rCTE Clang Tools Extra CHANG

[PATCH] D58603: Updated the documentation build instructions for the current CMake build system

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/docs/README.txt:6 +You can generate documentation in HTML format from files in +clang-tools-extra/docs by building the docs-clang-tools-html target. ilya-biryukov wrote: > ilya-biryukov wrote: > > C

[PATCH] D58603: Updated the documentation build instructions for the current CMake build system

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL354779: Updated the documentation build instructions for the current CMake build system (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Chang

[PATCH] D58611: Fixed typos in tests: s/CHEKC/CHECK/

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jsji, javed.absar, nemanjai. Herald added projects: clang, LLVM. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58611 Files: clang/test/CodeGenObjC/dllstorage.m

[PATCH] D58611: Fixed typos in tests: s/CHEKC/CHECK/

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC354785: Fixed typos in tests: s/CHEKC/CHECK/ (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D58611?vs=188143&id=188150#toc Repository: rC Clang CHANGES SIN

[PATCH] D58607: Moved clangd docs to a separate directory in preparation to restructure them into multiple files

2019-02-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE354786: Moved clangd docs to a separate directory in preparation to restructure them… (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D58607?vs=188131&id=1881

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:302 + if (const auto *CEArg = dyn_cast(Arg)) { +// Strip the ediable move constructor, it is presented in C++11/14 mode. +// e.g. Foo(Bar{1, 2}), the Ba

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:311 + } +} +if (CEArg->isStdInitListInitialization()) So `Foo(Bar{1, 2})` is not problematic, it is just that we can't tell it apart

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. Very nice testing approach! Comment at: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h:20 +/// Provides notifications for file changes in a directory. + +/// Invokes client-provided function on every filesystem event in the watched --

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:31 + Result, + "prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC", + ReplacementText); hokein wrote: > the message doesn't seem to expla

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr requested changes to this revision. gribozavr added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe2.rst:6 + +Checks if the required file flag ``O_CLOEXEC`` is present in the argum

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306 +if (CEArg->isElidable()) { + if (const auto *TempExp = CEArg->getArg(0)) { +if (const auto *Unwr

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40 +// FDRead. +// Currently used just for one-off termination signal. +struct SemaphorPipe

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:317 + + if (pipe(InotifyPollingStopperFDs) == -1) +return nullptr; Use pipe2() with O_CLOEXEC, to avoid leaking the file descriptors to child processes?

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306 +if (CEArg->isElidable()) { + if (const auto *TempExp = CEArg->getArg(0)) { +if (const auto *UnwrappedCE = Eugene.Zelenko wr

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:54 + +void e() { + int pipefd[2]; srhines wrote: > gribozavr wrote: > > How is `e` different from `a`? > `e` uses O_CLOEXEC properly with `pipe2()` and makes

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306 +if (CEArg->isElidable()) { + if (const auto *TempExp = CEArg->getArg(0)) { +if (const auto *UnwrappedCE = Eugene.Zelenko wr

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

2019-05-31 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306 +if (CEArg->isElidable()) { + if (const auto *TempExp = CEArg->getArg(0)) { +if (const auto *UnwrappedCE = Eugene.Zelenko wr

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:5 + +void f() { + int pipefd[2]; Please give the tests informative names in

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:472 if (Info.hasSourceManager()) checkFilters(Info.getLocation(), Info.getSourceManager()); } nik wrote: > gribozavr wrote: > > It seems like the `checkFilters` call

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:14 +int pipe(int pipefd[2]); +void noWarning() { + int pipefd[2]; noWarningForPipeInNamespace ? Comment a

[PATCH] D62845: [clang-tidy] Fix make-unique tests on C++2a.

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. I'm also not sure what the intent behind these tests is. Maybe the right fix is to add a constructor that can be called? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62845/new/ https://reviews.llvm.org/D62845 __

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:20 + pipe2(pipefd, O_NONBLOCK); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'pipe2' should use O_CLOEXEC where possible [android-cloexec-pipe2] + // CHECK-FIXES: pipe2(

[PATCH] D62845: [clang-tidy] Fix make-unique tests on C++2a.

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. I'd suggest to add a separate file that covers the exact language modes needed. The C++14 test that we have right now is about C++14-or-later, testing the availability of std::make_unique. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. Sorry for jumping in late, but renaming the declaration is not enough -- usages should also be updated; otherwise the developer is better off using a refactoring in their IDE or even a textual search and replace. Repository: rL LLVM CHANGES SINCE LAST ACTION htt

[PATCH] D62845: [clang-tidy] Fix make-unique tests on C++2a.

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. In D62845#1528887 , @hokein wrote: > In D62845#1528791 , @gribozavr wrote: > > > I'd suggest to add a separate file that covers the exact language modes > > needed. > > > > The C++14 test

[PATCH] D62845: [clang-tidy] Fix make-unique tests on C++2a.

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. In D62845#1529149 , @hokein wrote: > > I think we should be looking at the intent of the test rather than its name. > > > > The intent looks like testing how the check works when `std::make_unique` > > is available from the sta

[PATCH] D62845: [clang-tidy] Fix make-unique tests on C++2a.

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Looks great, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62845/new/ https://reviews.llvm.org/D62845 __

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:20 + pipe2(pipefd, O_NONBLOCK); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'pipe2' should use O_CLOEXEC where possible [andr

[PATCH] D61486: [Basic] Introduce active dummy DiagnosticBuilder

2019-06-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. Is this patch still needed? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61486/new/ https://reviews.llvm.org/D61486 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D63128: Fixed google-readability-casting test to work in c++17

2019-06-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. I'll commit this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63128/new/ https://reviews.llvm.org/D63128 _

[PATCH] D63128: Fixed google-readability-casting test to work in c++17

2019-06-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL363047: Fixed google-readability-casting test to work in c++17 (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: htt

[PATCH] D63127: [clang-tidy] Fixed checker for abseil to work in C++17 mode

2019-06-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:86 +.bind("ByAnyChar"), +expr(hasDescendant(cxxConstructExpr( + hasType(recordDecl(hasName("::absl::ByAnyChar"))), --

[PATCH] D63129: [clang-tidy] Fix invalid read on destruction

2019-06-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. This fix works. The alternative would have been to wrap these variables into llvm::ManagedStatic, just like the problematic TrueMatcherInstance in ASTMatchersInternal.cpp. Repository:

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

2019-06-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6455 +/// Matches expressions that match InnerMatcher after any elidable constructor are stripped off. +/// 80 columns Comment at: clang/include/clang

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

2019-06-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6455 +/// Matches expressions that match InnerMatcher after any elidable constructor +/// are stripped off. In C++17 copy elidable constructors are no longer being "Match

[PATCH] D63188: Fixed a crash in misc-redundant-expression ClangTidy checker

2019-06-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: ilya-biryukov. Herald added a project: clang. Herald added a subscriber: cfe-commits. It was trying to pass a dependent expression into constant evaluator. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D63188 Files:

[PATCH] D63188: Fixed a crash in misc-redundant-expression ClangTidy checker

2019-06-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL363133: Fixed a crash in misc-redundant-expression ClangTidy checker (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

2019-06-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:600 + EXPECT_TRUE(matches(code2, matcher2, LanguageMode::CXX11OrLater)); + EXPECT_TRUE(matches(code3, matcher3, LanguageMode::CXX11OrLater)); +} Please inline

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

2019-06-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:69 + CXX17OrLater, + CXX2AOrLater +}; Cxx2aOrLater? (no need to uppercase things) Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:159 +s

[PATCH] D63261: [clang-tidy] Fixed abseil-time-subtraction to work on C++17

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp:45 + match(callExpr(hasParent(varDecl())).bind("e"), + *Node, *Result.Context)) != nullptr; }

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL363262: Added AST matcher for ignoring elidable constructors (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https

[PATCH] D63263: [clang-tidy] Fixed abseil-duration-unnecessary-conversion tests for c++17

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL363263: [clang-tidy] Fixed abseil-duration-unnecessary-conversion tests for c++17 (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pr

[PATCH] D63262: [clang-tidy] Made abseil-upgrade-duration-conversions tests pass on c++17

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL363270: [clang-tidy] Made abseil-upgrade-duration-conversions tests pass on c++17 (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pr

[PATCH] D63261: [clang-tidy] Fixed abseil-time-subtraction to work on C++17

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL363272: [clang-tidy] Fixed abseil-time-subtraction to work on C++17 (authored by gribozavr, committed by ). Herald added a

[PATCH] D63253: [clang-tidy] Made abseil-faster-strsplit-delimiter tests pass on C++17

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL363273: [clang-tidy] Made abseil-faster-strsplit-delimiter tests pass on C++17 (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D63288: [clang-tidy] Generalize TransformerClangTidyCheck to take a rule generator.

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:33 +StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context)

[PATCH] D63288: [clang-tidy] Generalize TransformerClangTidyCheck to take a rule generator.

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:33 +StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), Rule(MakeRule(getLangOpts(), Options)) { + assert(llvm::all_of(Rule.Cases, [](co

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-06-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. Thanks! Please add tests in `./unittests/Tooling/ReplacementsYamlTest.cpp`. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:43 +ReplacementText.replace(lineBreakPos, 1, "\n\n"); +// Get the next occurrence from the current p

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

2019-07-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:25 + +struct [[gsl::Owner(int)]] OwnerWithConv { + OwnerWithConv(); xazax.hun wrote: > gribozavr wrote: > > Would be nice if the second pair of types had clearly relate

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

2019-07-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. In D64448#1581719 , @mgehre wrote: > In D64448#159 , @gribozavr wrote: > > > I don't know how various versions of libstdc++ differ. > > > The current implementation passed the (partial

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

2019-07-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.def:486 +TYPE_TRAIT_1(__is_gsl_owner, IsGslOwner, KEYCXX) +TYPE_TRAIT_1(__is_gsl_pointer, IsGslPointer, KEYCXX) KEYWORD(__underlying_type , KEYCXX) mgehre wrote: > griboz

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

2019-07-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. In D64448#1581866 , @mgehre wrote: > I didn't know whether they would differ, but the test tells me that they > don't differ significantly (i.e. in introducing new techniques). Okay -- I would prefer if you intentionally looke

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

2019-07-15 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. Please add tests to `llvm/tools/clang/unittests/Tooling/RecursiveASTVisitorTests/`. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2332 S->isSemanticForm() ? S->getSyntacticForm() : S, Queue)); TRY_TO(TraverseSynOrSemInitListExpr

[PATCH] D63954: Add lifetime categories attributes

2019-07-15 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4164 + let Content = [{ +When annotating a class ``O`` with ``[[gsl::Owner(T)]]``, then each function +that returns cv-qualified ``T&`` or ``T*`` is assumed to return a mgehre wro

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

2019-07-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. Is it true that all inline commands require an argument? For example, "\&" does not. Comment at: clang/test/Sema/warn-documentation.cpp:1030 +// The inline comments expect a string after the command. +// expected-warning@+1 {{'\a' command does not

[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

2019-07-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:25 + +static bool isAccessForVar(const Stmt *St, const VarDecl *Var) { + if (const auto *DRE = dyn_cast(St)) St => S "S" is a common abbreviation in the Clang codebase, "St"

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

2019-07-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2332 S->isSemanticForm() ? S->getSyntacticForm() : S, Queue)); TRY_TO(TraverseSynOrSemInitListEx

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

2019-07-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. Looks like a good idea to me. Regarding tests, I couldn't find existing tests that check order either. Seems like you'd need to make some minimal infrastructure for that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D649

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

2019-07-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thank you! Do you have commit access or would you like me to commit this patch for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64696/new/ https://reviews.llvm.org/D64696

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

2019-07-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:7095 auto *MTE = dyn_cast(L); + if (IsGslPtrInitWithGslTempOwner) { +Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) << DiagRange; I

[PATCH] D63954: Add lifetime categories attributes

2019-07-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: cfe/trunk/include/clang/Basic/AttrDocs.td:4252 + +The argument ``T`` is optional and currently ignored. +This attribute may be used by analysis tools and has no effect on code "and is currently ignored" even better:

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

2019-07-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6563 +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) { + if (auto *Conv = dyn_cast_or_null(Callee)) This looks like an ad-hoc rule. Is there a way to express it wit

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

2019-07-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/include/clang/Sema/Sema.h:6097 + + /// Add [[gsl::Owner]] and [[gsl::Pointer]] attributes for std:: types. + void addDefaultGslPointerAttribute(TypedefNameDecl *TD); It seems like this function does not add gsl

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

2019-08-05 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL367809: Adds a warning when an inline Doxygen comment has no argument (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

2019-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/AST/RecursiveASTVisitorTest.cpp:107 +} \ No newline at end of file Please add a newline. Repository: rG LLVM Githu

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

2019-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6581 +if (!Callee->getIdentifier()) { + auto OO = Callee->getOverloadedOperator(); + return OO == OverloadedOperatorKind::OO_Subscript || mgehre wrote: > xazax.hun wrote: > > I

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

2019-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Sema/Sema.h:6122 + /// std::container::iterator. \param UnderlyingRecord The record named by ND. + void addDefaultGslPointerAttrib

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

2019-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:7031 + LLVM_FALLTHROUGH; +case IndirectLocalPathEntry::DefaultInit: return Path[I].E->getSourceRange(); This change would be best committed

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

2019-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6572 +return false; + return llvm::StringSwitch(Callee->getName()) + .Cases("begin", "rbegin", "cbegin", "crbegin", true) xazax.hun wrote: > gribozavr wrote: > > Should we also che

[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

2019-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: docs/clang-tidy/checks/bugprone-infinite-loop.rst:12 +the loop condition is not changed. This check detects such loops. A loop is +considered as infinite if it does not have any loop exit statement (``break``, +``continue``, ``goto``,

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

2019-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h:102 - /// Returns nullptr if \param Path doesn't exist or isn't a directory. - /// Returns nullptr if OS kernel API told us we can't start watching. In such - /// case it

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

2019-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:283 /*waitForInitialSync=*/true); + if (!DW) return; plotfi wrote: > jkorous wrote: > > jkorous wrote: > > > IIUC this is silently dropping errors. We

<    1   2   3   4   5   6   7   8   9   10   >