[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-03-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D44602#1047574, @lebedev.ri wrote: > - No longer count variables declared in macro expansion. Please see FIXME, i > think this is too broad. Ping, thoughts? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44602 __

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-03-31 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 140547. lebedev.ri added a comment. - Rebased - Also test that `*&` contraption silences the warning. Repository: rC Clang https://reviews.llvm.org/D44883 Files: docs/ReleaseNotes.rst lib/Sema/SemaExpr.cpp test/SemaCXX/implicit-exception-spec.cp

[PATCH] D45128: [libcxx][test] Silence -Wself-assign diagnostics

2018-03-31 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: mclow.lists, EricWF. https://reviews.llvm.org/D44883 extends -Wself-assign to also work on C++ classes. These new warnings pop up in the test suite, so they have to be silenced. Please refer to the https://reviews.llvm.org/D45082 for

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-03-31 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 140554. lebedev.ri added a comment. - Revised release notes entry - Created https://reviews.llvm.org/D45128 with libcxx patch from stage-2 testing. Repository: rC Clang https://reviews.llvm.org/D44883 Files: docs/ReleaseNotes.rst lib/Sema/SemaExp

[PATCH] D45128: [libcxx][test] Silence -Wself-assign diagnostics

2018-03-31 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/std/language.support/support.types/byteops/and.assign.pass.cpp:30 -static_assert(noexcept(b &= b), "" ); +static_assert(noexcept(b &= (std::byte &)b), "" ); EricWF wrote: > Should Clang really be war

[PATCH] D45128: [libcxx][test] Silence -Wself-assign diagnostics

2018-03-31 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/std/language.support/support.types/byteops/and.assign.pass.cpp:30 -static_assert(noexcept(b &= b), "" ); +static_assert(noexcept(b &= (std::byte &)b), "" ); Quuxplusone wrote: > lebedev.ri wrote: > >

[PATCH] D45147: [clang] Implement Intercal compatibility feature

2018-04-01 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. testcase Please add docs, release notes entry, and most importantly, tests! Repository: rC Clang https://reviews.llvm.org/D45147 ___ cfe-commits mailing list cfe-co

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: aaron.ballman, rsmith, rtrieu, rjmccall, dblaikie. I have seen such a problem when reviewing https://reviews.llvm.org/D43341. https://godbolt.org/g/aJYcaa #include struct S {}; void test(S a) { std::move(a); } Sin

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D44883#1054326, @thakis wrote: > In https://reviews.llvm.org/D44883#1048751, @dblaikie wrote: > > > Historically Clang's policy on warnings was, I think, much more > > conservative than it seems to be today. There was a strong desire not to

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45163#1054925, @Quuxplusone wrote: > > I think it wouldn't be unreasonable to ask standard library maintainers to > > add `[[nodiscard]]` to `std::move` > > +1. Also `std::forward`, for sure. Basically any metaprogramming function > that

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45163#1054994, @Quuxplusone wrote: > `std::move` would definitely be special in this regard if there were a > pressing benefit to be gained — i.e., if people were currently getting bitten > by accidentally discarded calls of `std::move(x)

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 140787. lebedev.ri added a comment. - Rebased - Silence both of the diagnostics in an unevaluated context. - Fixed `-Wself-assign-field` preexisting problems: - False-positives on `volatile` stores. - Do not warn in template instantiations. - Handle a

[PATCH] D45128: [libcxx][test] Silence -Wself-assign diagnostics

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 140788. lebedev.ri added a comment. The diagnostic was adjusted not to warn in an unevaluated context. The remaining diff will have to remain, unless you want to disable `-Wself-assign` altogether for tests... If there are remaining problems with this dif

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45163#1055856, @rjmccall wrote: > Well, we should feel welcome to submit patches to the C++ library > implementations, I think. I can understand why Marshall is waiting to just > do this until he gets a comprehensive committee paper,

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/libcxx/diagnostics/force_nodiscard.fail.cpp:15 +// MODULES_DEFINES: _LIBCPP_FORCE_NODISCARD +#define _LIBCPP_FORCE_NODISCARD +#include <__config> Quuxplusone wrote: > What is the purpose of `_LIBCPP_FORCE_NODISCA

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45163#1056044, @rjmccall wrote: > It seems reasonable to ask libc++ to use > `__attribute__((warn_unused_result))` if `[[nodiscard]]` is unavailable. https://reviews.llvm.org/D45179 should hopefully add an **opt-in** define to allow doi

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45179#1056108, @Quuxplusone wrote: > > Opt-in is an enhancement. Of course, it would be nice to always default it > > to on, but as it was disscussed with @mclow.lists, this is simply not going > > to happen. This is the best we'll get. >

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45179#1056183, @rjmccall wrote: > Is Marshall arguing that the standard doesn't allow compilers to warn about > failing to use these function results prior to C++17? No. He was simply saying that people are opposed to having nodiscard at

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D44883#1054326, @thakis wrote: > In https://reviews.llvm.org/D44883#1048751, @dblaikie wrote: > > > Historically Clang's policy on warnings was, I think, much more > > conservative than it seems to be today. There was a strong desire not to

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 140934. lebedev.ri added a comment. Based on IRC disscussion, try to update the patch. The main problem is that gcc does not silence the warning produced by these attributes via `(void)` cast, unless it's `[[nodiscard]]` attribute :/ https://godbolt.org/g/

[PATCH] D45401: Fix 31480 - warn more aggressively when assignments get used as a boolean values

2018-04-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Test? https://reviews.llvm.org/D45401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45403: Make [[maybe_unused]] work with static data members

2018-04-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp:5 int I [[maybe_unused]]; - static int SI [[maybe_unused]]; // expected-warning {{'maybe_unused' attribute only applies to variables, functions, methods, types, enumerations, enum

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-04-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Hmm. Got back to this issue. Not reproducible with gcc-4.8.5 and gcc-4.9.3 in ubuntu 16.04 chroot. *Reproducible* with gcc-4.8.4 and gcc-4.9.2 in debian oldstable (Jessie) chroot. So one might assume that it was fixed. I did try to creduce the crasher, but not not muc

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Ping. @aaron.ballman ping, do you have any further thoughts on that macro false-negative? #define vardecl(type, name) type name; void variables_15() { // FIXME: surely we should still warn here? vardecl(int, a); vardecl(int, b); } // CHECK-MESSAGE

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-04-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D43779#1061444, @alexfh wrote: > In https://reviews.llvm.org/D43779#1061043, @lebedev.ri wrote: > > > Hmm. > > Got back to this issue. > > > > Not reproducible with gcc-4.8.5 and gcc-4.9.3 in ubuntu 16.04 chroot. > > *Reproducible* with gc

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290 +: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 0)), + MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {} alexfh wrote: > Maybe mak

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290 +: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 0)), + MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {} zinovy.nis wrote: > lebed

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

2018-04-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. @firolino do you plan on finishing this? https://reviews.llvm.org/D27621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D44883#1063003, @thakis wrote: > This landing made our clang trunk bots do an evaluation of this warning :-P > It fired 8 times, all false positives, and all from unit tests testing that > operator= works for self-assignment. > (https://c

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Thank you for working on this check! Comment at: clang-tidy/cppcoreguidelines/ConstCheck.h:60 + + std::unordered_map ValueCanBeConst; + std::unordered_map HandleCanBeConst; I'm guessing these should be `llvm::DenseMap`. Repositor

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This change had two different problems. Please watch the bots? Repository: rL LLVM https://reviews.llvm.org/D45405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:31 +!(isa(VD) || isa(VD)) && +!VD->getLocation().isMacroID()) + Info.Variables++; aaron.ballman wrote: > This isn't the restriction I was envisioning.

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 141912. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. - Update - Ignore GNU Statement Expressions too. - Slightly reword docs. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44602 Files: clang-tidy/readabilit

[PATCH] D44093: [BUILTINS] structure pretty printer

2018-04-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/CodeGen/dump-struct-builtin.c:1 +// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s + This should be ``` // RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s ``` or something Repo

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 141997. lebedev.ri added a reviewer: alexfh. lebedev.ri added a comment. After a quick IRC chat with @aaron.ballman, it was discovered that there has been a big misunderstanding: we don't need to ignore/exempt *all* the variables declared in macros, we

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 142003. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. - Drop stale comment - Switch to pre{in,de}crement Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44602 Files: clang-tidy/readability/FunctionSizeCheck.cp

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. I'm waiting for @mclow.lists to have the final say re this differential. In https://reviews.llvm.org/D45179#1057187, @Quuxplusone wrote: > In https://reviews.llvm.org/D45179#1056706, @lebedev.ri wrote: > > > gcc does not silence the warning produced by > > these attr

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Please always upload all patches with full context (`-U9`) Repository: rC Clang https://reviews.llvm.org/D45532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

2017-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 120285. lebedev.ri added a comment. - Rebased - Handle `-Wsystem-headers` properly, as per irc disscussion. It was suggested that `findMacroSpelling()` might be slow, and `isNullPointerConstant()` should be used, but i'm not sure any of the `NullPointerC

[PATCH] D39301: Ignore implicity casts for zero-as-null-pointer-constant warning

2017-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/SemaCXX/warn-zero-nullptr.cpp:29 + +// Shouldn't warn. +struct pr34362 { operator int*() { return nullptr; } }; Maybe wrap into a `namespace PR34362 {}` https://reviews.llvm.org/D39301 __

[PATCH] D39301: Ignore implicity casts for zero-as-null-pointer-constant warning

2017-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/Sema/Sema.cpp:441 return; - if (E->getType()->isNullPtrType()) + if (E->IgnoreImpCasts()->getType()->isNullPtrType()) return; erichkeane wrote: > aaron.ballman wrote: > > Do we also want to ignore pare

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

2017-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D38954#906900, @thakis wrote: > Can you build some large-ish codebase (say, LLVM) with and without this patch > and make sure that this doesn't measurably add to build perf? (With the > warning turned on, obviously.) > > Other than that, t

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

2017-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 120305. lebedev.ri added a comment. Rebased now that https://reviews.llvm.org/D39301 has landed. Repository: rL LLVM https://reviews.llvm.org/D38954 Files: docs/ReleaseNotes.rst lib/Sema/Sema.cpp test/SemaCXX/Inputs/warn-zero-nullptr.h test/Se

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

2017-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. > Hm, that's a lot of overhead. Granted, it's a synthetic benchmark, but I > think it'd be good to try this on some real codebase to make sure it really > is negligible overhead in real-world scenarios. Hm wait, i'm comparing apples and oranges here, my local build a

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

2017-10-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 120316. lebedev.ri added a comment. Add `Diags.isIgnored()` early return + move `C++11` early return earlier too. At least partially should help avoid the penalty of `findMacroSpelling()` I don't think i see any way to avoid `findMacroSpelling()` :/ Repos

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 120407. lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. Rebased. Tighten matchers. Add an assert that the function we matched does have the body (the check analyses the function bodies, so if there is no body, there is nothing to d

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-26 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:499 + functionDecl( + allOf(isDefinition(), unless(anyOf(isInstantiated(), isImplicit() + .bind("func"), aaron.ballman wrote: > Si

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a subscriber: thakis. lebedev.ri added a comment. That is my diagnostic, so i guess this is the time to reply :) In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote: > I dislike this change fairly strongly. > I would much rather pursue a clang-based solution (since cl

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D39149#910891, @bcain wrote: > In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote: > > > That is my diagnostic, so i guess this is the time to reply :) > > > ... > > > 3. In `-Wtautological-constant-compare`, ignore any comparisons

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D39149#911024, @rsmith wrote: > In https://reviews.llvm.org/D39149#910936, @smeenai wrote: > > > I'm thinking you could account for all possible type sizes in the existing > > (enabled by default) warning, and have a different warning for p

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 120890. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. Ping. Rebased. Silenced (via the `-w` flag in the `RUN` line) the `Fix conflicts with existing fix!` assertion since this differential does not actually cause it, but the te

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-31 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote: > I dislike this change fairly strongly. > I would much rather pursue a clang-based solution (since clang is being > unhelpful here) > Don't know if we can get one, though. @smeenai https://reviews.l

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

2017-10-31 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 121044. lebedev.ri added a comment. In https://reviews.llvm.org/D39462#912011, @rjmccall wrote: > The actual choice of integer type is not stable across targets any more than > the size is. In practice, people often don't use int and long, they use > st

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

2017-11-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 121743. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. In https://reviews.llvm.org/D39462#912011, @rjmccall wrote: > The actual choice of integer type is not stable across targets any more than > the size is. In practice, peo

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-11-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 122053. lebedev.ri added a comment. Ping. In https://reviews.llvm.org/D36836#889375, @aaron.ballman wrote: > Adding @dberlin for licensing discussion questions. @dberlin ping? I'm wondering if you had the chance to look at this? :) Repository: rL LL

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

2017-11-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D39462#917421, @rjmccall wrote: > So, that change makes this very interesting, because I think the right way of > looking at it is as the first in a larger family of warnings that attempt to > treat typedefs as if they were a much stronger

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

2017-11-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D39462#922826, @rjmccall wrote: > I don't speak for the entire project, but I'm not sure I'm interested in the > diagnostic you're actually offering to contribute here. It may produce a > warning on your specific test case, but I think it

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

2017-11-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D39462#922847, @rjmccall wrote: > In https://reviews.llvm.org/D39462#922844, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D39462#922826, @rjmccall wrote: > > > > > I don't speak for the entire project, but I'm not sure I'm interested

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-11-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Ping. In https://reviews.llvm.org/D36836#889375, @aaron.ballman wrote: > Adding @dberlin for licensing discussion questions. @dberlin ping? I'm wondering if you had the chance to look at this? :) Repository: rL LLVM https://reviews.llvm.org/D36836 __

[PATCH] D38921: [analyzer] LoopUnrolling: update the matched assignment operators

2017-11-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Not sure how `clang/docs/LibASTMatchersReference.html` is supposed to be updated. Comment at: include/clang/ASTMatchers/ASTMatchers.h:3931 + +/// \brief Matches all kind of assignment operators. +/// Maybe ``` -/// \brief Matches al

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

2017-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/CXX/over/over.match/over.match.funcs/p4-0x.cpp:22-24 + void lvalue() &; // expected-note 2 {{'lvalue' declared here}} + void const_lvalue() const&; + void rvalue() &&; // expected-note {{'rvalue' declared here}} --

[PATCH] D40242: Do not perform the analysis based warning if all warnings are ignored

2017-11-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. You probably want to add a test for this. https://reviews.llvm.org/D40242 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40242: Do not perform the analysis based warning if all warnings are ignored

2017-11-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D40242#930125, @ogoffart wrote: > I do not know how to add a test: there is no real visible change for clang. > It just should be faster when passing "-w". Oh right, i was thinking of something else there. Comment at:

[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

2017-11-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 123827. lebedev.ri marked an inline comment as done. lebedev.ri retitled this revision from "[clang-tidy] Implement readability-function-cognitive-complexity check" to "[clang-tidy] Implement sonarsource-function-cognitive-complexity check". lebedev.ri add

[PATCH] D139267: Supporting tbaa.struct metadata generation for bitfields

2022-12-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added reviewers: wsmoses, jdoerfert. lebedev.ri added a comment. Certainly needs tests. I'm not sure if anyone actually knows this area of LLVM, though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139267/new/ https://reviews.llvm.org/D

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

2022-12-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang/test/CodeGen/windows-seh-EHa-CppCondiTemps.cpp:3 +// FIXME: this check appears to be miscompiled? +// XFAIL: * tentzen wrote: > lebedev.ri wrote: > > This test broke once we always started adding (outermost) U

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

2022-12-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked 2 inline comments as done. lebedev.ri added inline comments. Comment at: clang/test/CodeGen/windows-seh-EHa-CppCondiTemps.cpp:3 +// FIXME: this check appears to be miscompiled? +// XFAIL: * efriedma wrote: > lebedev.ri wrote: > > tentzen wrote

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

2022-12-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. (ping, maybe) In D138958#3968118 , @lebedev.ri wrote: > I've been thinking, and it seems to me that explicitly opting into > `__attribute__((nounwind))`-provided semantics should override > the `__attribute__((nothrow))`/`noe

[PATCH] D139197: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace

2022-12-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D139197#3977370 , @carlosgalvezp wrote: > @lebedev.ri If you are happy with the patch, would you mind approving it? Or > do you have additional comments that should be addressed? The patch does not > need to be approved b

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

2022-12-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: clang/test/CodeGen/windows-seh-EHa-CppCondiTemps.cpp:3 +// FIXME: this check appears to be miscompiled? +// XFAIL: * tentzen wrote: > lebedev.ri wrote: > > efriedma wrote

[PATCH] D139006: [UpdateTestChecks] Match define for labels

2022-12-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a subscriber: nikic. lebedev.ri added a comment. In D139006#3981326 , @sebastian-ne wrote: > I guess this is fine to merge. I’ll leave it for a day in case someone has > more comments. I guess, we could be pedantic and post an RFC to d

[PATCH] D139647: [opt] Disincentivize new tests from using old pass syntax

2022-12-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. @aeubanks Thank you for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139647/new/ https://reviews.llvm.org/D139647 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

2022-12-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This is missing a test, like the original commit mentioned. Why can you not just use the the split variant, `-X clang ...`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139717/new/ https://reviews.llvm.org/D139717

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

2022-12-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Last(?) ping of the year? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138958/new/ https://reviews.llvm.org/D138958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

2022-12-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Last(?) ping of the year? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137381/new/ https://reviews.llvm.org/D137381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D31252: Clang-tidy: add readability-compound-statement-size check.

2017-03-22 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang-tools-extra. Herald added a subscriber: mgorny. This check is quite similar to the readability-function-size check. But it is more generic. It works for each compound statement, excluding the ones directly related to the function,

[PATCH] D31252: Clang-tidy: add readability-compound-statement-size check.

2017-03-22 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Do note that clang-tidy/readability/CompoundStatementSizeCheck.cpp is *very* similar to the clang-tidy/readability/FunctionSizeCheck.cpp I'm not sure if it makes sense to have such duplication. But i'm also not sure yet how to deduplicate them. But in the long run, i

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 92774. lebedev.ri added a comment. Addressing review notes. https://reviews.llvm.org/D31252 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/CompoundStatementSizeCheck.cpp clang-tidy/readability/CompoundStatementSizeCheck.h cla

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 92775. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. No changes compared to v2, just correctly rebased the master branch now. https://reviews.llvm.org/D31252 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readabili

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D31252#708209, @Eugene.Zelenko wrote: > Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). Done > Will be good idea to run Clang-tidy modernize and readability checks over new > code. I did run them (`-checks=*`

[PATCH] D7375: [clang-tidy] Assert related checkers

2017-03-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/clang-tidy/misc-assert-side-effect.cpp:67 + + assert(freeFunction()); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() with side effect Is it intentional that the check also warns on free functions

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-07-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Will land this shortly.. Repository: rL LLVM https://reviews.llvm.org/D33102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-07-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 105103. lebedev.ri added a comment. Rebased before commit. Repository: rL LLVM https://reviews.llvm.org/D33102 Files: docs/ReleaseNotes.rst lib/Sema/SemaCast.cpp test/Sema/warn-cast-qual.c test/SemaCXX/warn-cast-qual.cpp Index: test/SemaCXX/w

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

2017-07-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. @thakis ping? https://reviews.llvm.org/D32914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

2017-07-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. What will this check say about the following code? c++ void function(void) { char c = 'x'; int *ip = reinterpret_cast(&c); } The clang'g `-Wcast-align` does not warn: https://godbolt.org/g/hVwD5S Repository: rL LLVM https://reviews.llvm.org/D33826

[PATCH] D35068: Detect usages of unsafe I/O functions

2017-07-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This does not do anything more than traversing the AST, shouldn't this be a clang-tidy check? Also, i suspect CERT-MSC24-C might be relevant ==

[PATCH] D33365: [clang-tidy] misc-assertion-count: A New Check

2017-07-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D33365#775916, @alexfh wrote: > In https://reviews.llvm.org/D33365#775880, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D33365#775860, @alexfh wrote: > > > > > I guess, this check should go to `readability` or elsewhere, but > > > d

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

2017-07-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. PR #33771 filled. IMHO the problems with this diagnostic should be resolved before 5.0 https://reviews.llvm.org/D32914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35472: Implement P0463R1: "Endian just Endian"

2017-07-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: include/type_traits:4740 +#if _LIBCPP_STD_VER > 14 +enum class endian (Apologies for double commenting, did not notice that it was in phab until after replying) I'm probably wrong, but if this is a C++2a proposal,

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. How does this relate to the gcc behavior? I suspect not everyone would want to have this relaxed `-Wshadow` mode. Perhaps it could be hidden under some new flag, which is not going to be automatically enabled by `-Weverything`. Like, `-Wshadow-in-macros` which does not

[PATCH] D41815: [clang-tidy] implement check for goto

2018-01-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:20 + +AST_MATCHER(GotoStmt, isForwardJumping) { + JonasToth wrote: > lebedev.ri wrote: > > It would be nice to have it in standard ASTMatchers, i believe it will be > >

[PATCH] D41815: [clang-tidy] implement check for goto

2018-01-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:21 +AST_MATCHER(GotoStmt, isForwardJumping) { + return Node.getLocStart() < Node.getLabel()->getLocStart(); +} Hm, on a second thought, i think this will have false-pos

[PATCH] D41815: [clang-tidy] implement check for goto

2018-01-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:21 +AST_MATCHER(GotoStmt, isForwardJumping) { + return Node.getLocStart() < Node.getLabel()->getLocStart(); +} JonasToth wrote: > lebedev.ri wrote: > > Hm, on a second

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

2018-01-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D41976#975659, @tvanslyke wrote: > Here are the results: > > Comparing old-copymove.json to new-copymove.json > BenchmarkTime CPU Time Old > Time New CPU Old CPU New > > -

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

2018-01-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Thank you for working on this! Some thoughts below. Comment at: clang-tidy/readability/CMakeLists.txt:31 UniqueptrDeleteReleaseCheck.cpp + UnnecessaryIntermediateVarCheck.cpp Please upload patches with full context (`-U99`)

[PATCH] D41455: [ASTMatchers] Add isNoReturn() match narrower for FunctionDeclarations

2018-01-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 130220. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. - rebased - finally updated docs - found a new bug in doc generator https://bugs.llvm.org/show_bug.cgi?id=35989 Going to finally land this. Repository: rC Clang https:

[PATCH] D41455: [ASTMatchers] Add isNoReturn() match narrower for FunctionDeclarations

2018-01-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 130221. lebedev.ri added a comment. Update extra newline that prevented full documentation generation :/ Repository: rC Clang https://reviews.llvm.org/D41455 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/AS

[PATCH] D41455: [ASTMatchers] Add isNoReturn() match narrower for FunctionDeclarations

2018-01-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 130228. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Address review note: reduce amount of examples in documentation. Repository: rC Clang https://reviews.llvm.org/D41455 Files: docs/LibASTMatchersReference.html include

[PATCH] D42213: [ASTMatchers] [NFC] Fix code examples

2018-01-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Don't forget that you also need to regenerate the HTML docs: $ cd docs/tools # yes, cd $ ./dump_ast_matchers.py Repository: rC Clang https://reviews.llvm.org/D42213 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D42273: Add hasTrailingReturn AST matcher

2018-01-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:5904 +return F->hasTrailingReturn(); + return false; +} There are no negative tests in the unittest that cover this false path. https://reviews.llvm.org/D42273 ___

<    16   17   18   19   20   21   22   23   >