[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 170045. lebedev.ri marked 6 inline comments as done. lebedev.ri added a comment. - Apply minor wording nits. - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not **anything** else (not even `llu`). Repository: rCTE Clang Tools Extra https:/

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D53372#1268207, @curdeius wrote: > In https://reviews.llvm.org/D53372#1267728, @lebedev.ri wrote: > > > I think it would be good to add some more explanation as to *why* that > > `else` has to be kept. > > > Do you mean add a comment in the

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 170065. lebedev.ri marked 6 inline comments as done. lebedev.ri added a comment. - Address nits - Proper default for CPPCG - Don't show record name. I indeed don't have a strong case here, it seems to be more useful to dump that info, but maybe not? ¯\_(ツ)

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D52771#1268367, @aaron.ballman wrote: > LGTM aside from a minor nit. Thank you for the review! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52771 ___ cfe-commits mailing l

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote: > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > > > - Apply minor wording nits. > > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not > > **anything** else (not even `ll

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote: > In https://reviews.llvm.org/D52670#1268372, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote: > > > > > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D52670#1268572, @aaron.ballman wrote: > In https://reviews.llvm.org/D52670#1268569, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote: > > > > > I talked to someone at CERT responsible for maintaining

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 170108. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. CERT: also handle "lu", "llu". Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 170109. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Don't use `auto` in `getModuleOptions()`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52771 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyMo

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

2018-10-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 170112. lebedev.ri added a comment. Rebased to fix a merge conflict in release notes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52771 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/misc/CMakeLists

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. @rsmith ping. Repository: rC Clang https://reviews.llvm.org/D50250 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53595: [C++17] Reject shadowing of capture by parameter in lambda

2018-10-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: include/clang/Sema/Sema.h:5584 + void addLambdaParameters( + const SmallVectorImpl &Captures, + CXXMethodDecl *CallOperator, Scope *CurScope); Maybe this should be `ArrayRef Captures` instead? Repository

[PATCH] D50251: [compiler-rt][ubsan] Implicit Conversion Sanitizer - integer sign change - compiler-rt part

2018-10-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 170953. lebedev.ri added a comment. Herald added a subscriber: cfe-commits. Rebased. The clang part https://reviews.llvm.org/D50250 finally got reviewed, so i'm wondering if someone could review this, please? :) Repository: rCTE Clang Tools Extra http

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-10-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 170952. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. In https://reviews.llvm.org/D50250#1273415, @rsmith wrote: > Just some minor nits. YES! Thank you for the [long-awaited] review! Addressed review notes. The compiler-rt par

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-10-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1036 +return; + // That's it. We can't rule out any more cases with the data we have. + rsmith wrote: > lebedev.ri wrote: > > rsmith wrote: > > > rsmith wrote: > > > > lebedev.ri wrot

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

2018-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. ping Repository: rC Clang https://reviews.llvm.org/D52695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53719: [clang-tidy] UppercaseLiteralSuffixCheck: bugfix: don't cripple substNonTypeTemplateParmExpr

2018-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun. lebedev.ri added a project: clang-tools-extra. Herald added subscribers: chrib, rnkovacs, kristof.beyls. The previous, clearly broken output: /tmp/test.cpp:4:17: warning: integer l

[PATCH] D53719: [clang-tidy] UppercaseLiteralSuffixCheck: bugfix: don't cripple substNonTypeTemplateParmExpr

2018-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D53719#1276208, @JonasToth wrote: > What do you mean by "general way to avoid"? Templates? I'm basically asking "is the `unless(hasParent(substNonTypeTemplateParmExpr()))` fix correct? or is there something else i'm missing?" Repositor

[PATCH] D53723: [clang-tidy] UppercaseLiteralSuffixCheck: bugfix: ignore implicit code

2018-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun. lebedev.ri added a project: clang-tools-extra. Herald added a subscriber: rnkovacs. lebedev.ri added a dependency: D53719: [clang-tidy] UppercaseLiteralSuffixCheck: bugfix: don't cri

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

2018-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171172. lebedev.ri added a comment. Reverted in https://reviews.llvm.org/rL345305 due to multiple apparently-lurking bugs. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cer

[PATCH] D53719: [clang-tidy] UppercaseLiteralSuffixCheck: bugfix: don't cripple substNonTypeTemplateParmExpr

2018-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri abandoned this revision. lebedev.ri added a comment. Reverted the check, reopened https://reviews.llvm.org/D52670. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53719 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D53723: [clang-tidy] UppercaseLiteralSuffixCheck: bugfix: ignore implicit code

2018-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri abandoned this revision. lebedev.ri added a comment. Reverted the check, reopened https://reviews.llvm.org/D52670. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53723 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

2018-10-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171279. lebedev.ri added a comment. This revision is now accepted and ready to land. Finally finished creducing the issue i wasn't able to understand even after https://reviews.llvm.org/D53719+https://reviews.llvm.org/D53723, and it turned out to be `hasPa

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:54 + +namespace { + Extend the namespace /\ above, so that function is also covered? Comment at: clang-tidy/readability/ConstValueReturnCheck.h:19-20

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

2018-10-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Rerun on clang-tools-extra+lld+clang codebase, nothing broken. I'm gonna reland. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

2018-10-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/clang-tidy/readability-isolate-declaration.cpp:233 + int member1, member2; + // TODO: Transform FieldDecl's as well +}; Comment is misleading. Transform == fixit, at least for me. But they are not even diagnose

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: aaron.ballman, JonasToth, alexfh, hokein, xazax.hun. lebedev.ri added a project: clang-tools-extra. Herald added subscribers: rnkovacs, mgorny. lebedev.ri edited the summary of this revision. PR39224

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/clang-tidy/misc-avoid-c-arrays.cpp:14 + +template +class array { JonasToth wrote: > Please add a case with templates, where `T` itself is the array type, maybe > there are other fancy template tricks that could

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-27 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171393. lebedev.ri marked 9 inline comments as done. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. - Moved into `modernize` - Detect based on `TypeLoc`, which means we now catch *everything*, including `using`-declarations. No

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-28 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171423. lebedev.ri added a comment. Herald added subscribers: kbarton, nemanjai. In https://reviews.llvm.org/D53771#1278177, @Eugene.Zelenko wrote: > Will be good idea to add aliases at least for existing modules. Added CPPCG, HICPP aliases. Repository

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-28 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171425. lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. Docs fixes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53771 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCore

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-28 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/cppcoreguidelines-avoid-c-arrays.rst:6 +cppcoreguidelines-avoid-c-arrays += + Eugene.Zelenko wrote: > Please make same length as header. W

[PATCH] D53808: [clangd] Don't collect refs from non-canonical headers.

2018-10-29 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Please add a test with `#pragma once`, which should still be counted as a header guard. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

2018-10-29 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:67 + + diag(MD->getLocation(), DiagnosticMessage); +} And once again, i'm going to bitch about useless diagnostic messages: ``` [1/357] Building CXX object src/CMakeF

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-29 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: JonasToth, aaron.ballman, hokein, xazax.hun, alexfh. lebedev.ri added a project: clang-tools-extra. Herald added subscribers: rnkovacs, kbarton, nemanjai. The macro may not have location (or more generally, the location may not exist),

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-29 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D53817#1278916, @JonasToth wrote: > How does this patch change the behaviour for macros without location? It doesn't. > They will be diagnosed at the beginning of the TU? > It is probably better to ignore these kind of macros, as there

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-29 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D53817#1278970, @aaron.ballman wrote: > In https://reviews.llvm.org/D53817#1278933, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D53817#1278916, @JonasToth wrote: > > > > > How does this patch change the behaviour for macros without

[PATCH] D53837: [clang] Move two utility functions into SourceManager

2018-10-29 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: rsmith, aaron.ballman. lebedev.ri added a project: clang. Herald added subscribers: kbarton, nemanjai. So we can keep that not-so-great logic in one place. Repository: rC Clang https://reviews.llvm.org/D53837 Files: include/clan

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-29 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171583. lebedev.ri added a comment. Add an opt-in option to diagnose such command-line-based macros. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53817 Files: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp clang-tidy/cppcoreguide

[PATCH] D53837: [clang] Move two utility functions into SourceManager

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D53837#1279891, @rsmith wrote: > Thanks, LG. I bet there's a bunch more places we could change to call these > two. Thank you for the review. Repository: rL LLVM https://reviews.llvm.org/D53837

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171683. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Better option name. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53817 Files: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp clang-tidy/cppcoreguide

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:30 + +.. option:: IgnoreLocationless + JonasToth wrote: > I think the another option name would fit better: how about > `IgnoreCommandLineMacros` or the like?

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171688. lebedev.ri added a comment. Rebased, NFC. Repository: rC Clang https://reviews.llvm.org/D50250 Files: docs/ReleaseNotes.rst docs/UndefinedBehaviorSanitizer.rst include/clang/Basic/Sanitizers.def lib/CodeGen/CGExprScalar.cpp test/Code

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D53817#1280405, @JonasToth wrote: > LGTM Thank you for the review! > @aaron.ballman do you have more comments? If there are no more comments, i'll land this in a few (~3) hours. Repository: rCTE Clang Tools Extra https://reviews.llv

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 "cppcoreguidelines-c-copy-assignment-signature"); +CheckFactories.registerCheck( +"cppcoreguidelines-avoid-c-arrays"); JonasToth wro

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171747. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. Address review notes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53771 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelin

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

2018-10-31 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D52695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Either this is a NFC change with just tests, or the actual fix is missing. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56644/new/ https://reviews.llvm.org/D56644 ___ cfe-co

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D56644#1355436 , @Quolyk wrote: > For now, I just added tests. I have several questions, as I'm beginner > (`ContainerSizeEmptyCheck.cpp`). > > 1. Do I have to extend `ValidContainer`, so it recognises `::std::string` > wit

[PATCH] D56646: [OpenCL] opencl-c.h: read_image*(): sampler-less, and image{1, 2}d_array_t variants are OpenCL-1.2+, mark them as such

2019-01-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: yaxunl, Anastasia, echuraev, asavonic. lebedev.ri added a project: clang. Refer to `6.11.13.2 Built-in Image Functions` , and `9.6.8 Image Read and Write Functions`

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Seems to look good. Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:65 hasType(references(ValidContainer), -callee(cxxMethodDecl(hasName("size"))), WrongUse, +

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:34 return; const auto ValidContainer = qualType(hasUnqualifiedDesugaredType( ``` auto ContainerLenghtFuncNames = anyOf(hasName("size"), hasName("length"));

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

2019-01-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options comes into mind. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54881/new/ https://reviews.llvm.org/D54881 ___ cfe-commits

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

2019-01-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options comes into mind. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40988/new/ https://reviews.llvm.org/D40988 ___ cfe-commits

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

2019-01-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 181608. lebedev.ri added a comment. Rebased before commit, NFC. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54589/new/ https://reviews.llvm.org/D54589 Files: docs/ReleaseNotes.rst docs/UndefinedBehaviorSanitizer.rst

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D56424#1357471 , @MyDeveloperDay wrote: > In D56424#1356959 , @karepker wrote: > > > Hi all, ping on this patch. I've addressed all comments to the best of my > > ability. Is there a

[PATCH] D56646: [OpenCL] opencl-c.h: read_image*(): sampler-less, and image{1,2}d_array_t variants are OpenCL-1.2+, mark them as such

2019-01-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D56646#1357650 , @Anastasia wrote: > LGTM! Thank you for the review. In D56646#1357650 , @Anastasia wrote: > Overall I feel we need to find some way to test the header better...

[PATCH] D56731: Add -Wimplicit-ctad warning to diagnose CTAD on types with no user defined deduction guides.

2019-01-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:1054 + +def ImplicitCTADUsage : DiagGroup<"implicit-ctad">; Should this be in some group? Alternatively, would it make sense to add it to new `-Wctad` group?

[PATCH] D56786: [ASTMatchers] Changes to `CXXMemberExpr` matchers.

2019-01-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D56786#1359903 , @ymandel wrote: > In D56786#1359879 , @steveire wrote: > > > Can you break this up into multiple commits? > > > Sure, but any suggestions on granularity? E.g. i can s

[PATCH] D56811: [Mem2Reg] Enable promotion for bitcastable load/store values

2019-01-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. The diff does not seem to match the subject. (also, probably should go do llvm-commits) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56811/new/ https://reviews.llvm.org/D56811 ___ cfe-com

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

2019-01-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added subscribers: cfe-commits, lebedev.ri. lebedev.ri added a comment. Please always subscribe lists. The reviews that happen without lists are null and void. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56849/new/ https://reviews.llvm.org/D56849

[PATCH] D56850: [ASTMatchers] Add tests for assorted `CXXMemberCallExpr` matchers.

2019-01-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added subscribers: cfe-commits, lebedev.ri. lebedev.ri added a comment. Please always subscribe lists. The reviews that happen without lists are null and void. Is this testing pre-existing behavior, and thus NFC? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56850/new/ https

[PATCH] D56851: [ASTMatchers] Adds `CXXMemberCallExpr` matcher `invokedAtType`.

2019-01-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added subscribers: cfe-commits, lebedev.ri. lebedev.ri added a comment. Please always subscribe lists. The reviews that happen without lists are null and void. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56851/new/ https://reviews.llvm.org/D56851

[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

2019-01-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Some nits. Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:21 AST_MATCHER(CXXRecordDecl, hasMethods) { + for (const auto &Method

[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

2019-01-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. LG other than two nits, thank you! Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:25 AST_MATCHER(CXXRecordDecl, hasNonStaticMethod) { + r

[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

2019-01-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst:9 -Finds classes that contain non-static data members in addition to non-static -member functions and diagnose all data members declared with a non-``public``

[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

2019-01-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst:9 -Finds classes that contain non-static data members in addition to non-static -member functions and diagnose all data members declared with a non-``public``

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Two passing-by remarks: 1. I would *love* for this check to produce less cryptic reports :) It currently does not output any details whatsoever. It's really unhelpful. 2. It would be great for it to be generalized to not only detect the escaping of exceptions out of

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D57100#1367817 , @JonasToth wrote: > In D57100#1367813 , @lebedev.ri > wrote: > > > 2. It would be great for it to be generalized to not only detect the > > escaping of exceptions ou

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:35 + std::vector FunctionsThatShouldNotThrowVec = + utils::options::parseStringList(RawFunctionsThatShouldNotThrow); FunctionsThatShouldNotThrow.insert(FunctionsThatShouldNotThrowV

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

2019-01-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: sbenza, bkramer, pcc, klimek, hokein. lebedev.ri added a project: OpenMP. lebedev.ri added a child revision: D57113: [clang-tidy] openmp-use-default-none - a new module and a check. `OMPClause` is the base class, it is not descendant f

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: JonasToth, aaron.ballman, alexfh, xazax.hun, hokein. lebedev.ri added projects: clang-tools-extra, OpenMP. Herald added subscribers: guansong, rnkovacs, mgorny. Finds OpenMP directives that are allowed to contain `default` clause, but e

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 183160. lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. Fix documentation nits. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57113/new/ https://reviews.llvm.org/D57113 Files: clan

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:35 + std::vector FunctionsThatShouldNotThrowVec = + utils::options::parseStringList(RawFunctionsThatShouldNotThrow); FunctionsThatShouldNotThrow.insert(FunctionsThatShouldNotThrowV

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:35 + std::vector FunctionsThatShouldNotThrowVec = + utils::options::parseStringList(RawFunctionsThatShouldNotThrow); FunctionsThatShouldNotThrow.insert(FunctionsThatShouldNotThrowV

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Hmm, i think this diff is incorrect. Doesn't it also include the changes from D57100 ? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57108/new/ https://reviews.llvm.org/D57108 ___

[PATCH] D54654: Correct CreateAlignmentAssumption to check sign and power-of-2 (Clang Part)

2019-01-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri abandoned this revision. lebedev.ri added a comment. As disscussed in D54653 (https://reviews.llvm.org/D54653#1318964), this differential will be simply unneeded, that review will address the issue too, so abandoning. Repository: rC Clang CHANGES

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

2019-01-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D56624#1369626 , @yln wrote: > In D56624#1368966 , @lebedev.ri > wrote: > > > Please revert this. > > First, this wasn't reviewed. > > Second, the lists weren't subscribed. > > > I

[PATCH] D57175: [NFC][clang] Test updates for CreateAlignmentAssumption() changes in D54653

2019-01-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. Depends on D54653 . Repository: rC Clang https://reviews.llvm.org/D57175 Files: test/CodeGen/alloc-align-attr.c test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function-variable.cpp Index: test/CodeGe

[PATCH] D57175: [NFC][clang] Test updates for CreateAlignmentAssumption() changes in D54653

2019-01-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Self-accepting since D54653 got reviewed. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57175/new/ https://reviews.ll

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. To me, modulo the `"IgnoredExceptions"` change, this seems like a straight-forward refactoring. Comment at: clang-tidy/utils/ExceptionAnalyzer.h:25 +/// give the possibility of an exception. +class ExceptionAnalyzer { +public: What

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked 4 inline comments as done. lebedev.ri added inline comments. Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:51 +///instead of the actual OpenMPClauseKind. +AST_MATCHER_P(OMPExecutableDirective, isAllowedToContainClause, + OpenMPClause

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked 4 inline comments as done and an inline comment as not done. lebedev.ri added a comment. @JonasToth thanks for taking a look BTW! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57113/new/ https://reviews.llvm.org/D57113 _

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

2019-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked an inline comment as done. lebedev.ri added a comment. @ABataev Thank you for taking a look! Comment at: lib/AST/ASTTypeTraits.cpp:114 +#define OPENMP_CLAUSE(Name, Class) \ +case OMPC_##Name: return ASTNodeKind(

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked 2 inline comments as done. lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/openmp-use-default-none.rst:12 +being implicitly determined, and thus forces developer to be explicit about the +desired data scoping for each variable. + ---

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked 2 inline comments as not done. lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/openmp-use-default-none.rst:12 +being implicitly determined, and thus forces developer to be explicit about the +desired data scoping for each variable. + ---

[PATCH] D57249: [clang-tidy] fix unit tests for dropped _Float16 support in X86

2019-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/clang-tidy/readability-uppercase-literal-suffix-float16.cpp:2 +// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -target aarch64-linux-gnu -I %S + +#include "readability-uppercase-literal-suffix.h" -

[PATCH] D57249: [clang-tidy] fix unit tests for dropped _Float16 support in X86

2019-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. @JonasToth @erichkeane thank you for taking care of this. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57249/new/ https://reviews.llvm.org/D57249 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

2019-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked an inline comment as done and an inline comment as not done. lebedev.ri added inline comments. Comment at: lib/AST/ASTTypeTraits.cpp:114 +#define OPENMP_CLAUSE(Name, Class) \ +case OMPC_##Name: return ASTNodeKind(

[PATCH] D57280: [clang][OpenMP] OMPFlushClause is synthetic, no such clause exists

2019-01-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a reviewer: ABataev. lebedev.ri added a project: OpenMP. Herald added subscribers: arphaman, guansong. As discussed in https://reviews.llvm.org/D57112#inline-506781, 'flush' clause does not exist in the OpenMP spec, it can not be specified, and `

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

2019-01-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked 5 inline comments as done. lebedev.ri added inline comments. Comment at: lib/AST/ASTTypeTraits.cpp:114 +#define OPENMP_CLAUSE(Name, Class) \ +case OMPC_##Name: return ASTNodeKind(NKI_##Class); +#include "clang/Bas

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

2019-01-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 183690. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Rebased ontop of D57280 , use `const auto*`. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/ https:

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

2019-01-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 183692. lebedev.ri added a comment. Store OpenMP clause spelling in the `ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[]`, not the stringified clang AST class name. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:49-50 +/// +/// FIXME: would be better to pass the actual class name (e.g. OMPDefaultClause) +///instead of the actual OpenMPClauseKind. +AST_MATCHER_P(OMPExecutableDirective, isAllow

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 183694. lebedev.ri marked 28 inline comments as done. lebedev.ri added a comment. Addressed review notes; found a way to use the `OMPClause` matcher in `isAllowedToContainClause()`, as opposed to passing the `OpenMPClauseKind`. Repository: rCTE Clang To

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 183699. lebedev.ri added a comment. Adjust a few comments. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57113/new/ https://reviews.llvm.org/D57113 Files: clang-tidy/CMakeLists.txt clang-tidy/ClangTidyFo

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 183701. lebedev.ri added a comment. Herald added a subscriber: arphaman. Mention new module in `docs/clang-tidy/index.rst`. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57113/new/ https://reviews.llvm.org/D5

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

2019-01-27 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/AST/ASTTypeTraits.cpp:40-43 + { NKI_None, "OMPClause" }, +#define OPENMP_CLAUSE(TextualSpelling, DERIVED) \ + {NKI_OMPClause, #TextualSpelling}, +#include "clang/Basic/OpenMPKinds.def" ---

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-27 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added subscribers: sbenza, klimek. lebedev.ri added a comment. In D57113#1372891 , @JonasToth wrote: > LGTM with the few language nits. > The new matchers look better and are more matcher style, gj :) Thank you for the review! I'm not quite s

[PATCH] D57280: [clang][OpenMP] OMPFlushClause is synthetic, no such clause exists

2019-01-28 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D57280#1373547 , @ABataev wrote: > LG with a nit Thank you for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57280/new/ https://reviews.llvm.org/D57280 _

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