whisperity added a comment.
In https://reviews.llvm.org/D53974#1294385, @ztamas wrote:
> I also tested on LLVm code.
> The output is here:
> https://gist.github.com/tzolnai/fe4f23031d3f9fdbdbf1ee38abda00a4
>
> I found 362 warnings.
>
> Around 95% of these warnings are similar to the next exampl
Author: d0k
Date: Sun Nov 11 03:09:58 2018
New Revision: 346608
URL: http://llvm.org/viewvc/llvm-project?rev=346608&view=rev
Log:
[clangd] Make ClangdFuzzer compile again.
Modified:
clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp
Modified: clang-tools-extra/trunk/clangd/fuzzer/ClangdF
bobsayshilol added a comment.
In https://reviews.llvm.org/D53263#1294477, @kristina wrote:
> Huge apologies, it seems I can't get this to patch cleanly against my fork
> and therefore can't test it before committing, which is something I generally
> always do. I'll leave it to someone else. Aga
vmiklos updated this revision to Diff 173554.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readability/RedundantPreprocessorCheck.h
docs/Rele
vmiklos marked an inline comment as done.
vmiklos added a comment.
> As it stands, I feel like this check is a bit too simplistic to have much
> value, but if you covered some of these other cases, it would alleviate that
> worry for me.
I've now added support for detecting inverted conditions
jankratochvil created this revision.
jankratochvil added a reviewer: clang.
jankratochvil added a project: clang.
With `z3-4.8.1` as found in Fedora Rawhide (future Fedora 30) as
`z3-devel-4.8.1-1.fc30.x86_64`:
../tools/clang/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp: In function
'void
vmiklos updated this revision to Diff 173556.
vmiklos marked an inline comment as done.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readabilit
vmiklos added a comment.
In https://reviews.llvm.org/D54349#1294600, @vmiklos wrote:
> Let's see if that works in practice.
I've implemented this now, please take a look. (Extended test + docs as well,
as usual.) Thanks!
https://reviews.llvm.org/D54349
xbolva00 added a comment.
Since no futher comments, i think it is ok.
After day or two I will commit it.
https://reviews.llvm.org/D52835
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
ztamas updated this revision to Diff 173559.
ztamas added a comment.
- Add new test cases based on findings in LibreOffice code base
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
c
ztamas added a comment.
>> Also found some new false positives related to macros. The number of all
>> false positives is around 38, which is still seems good to me.
>
> I would be very interested why these false positives are created. Is there a
> way to avoid them and maybe it makes sense to
ztamas updated this revision to Diff 173560.
ztamas added a comment.
- Fix comment
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
JonasToth added inline comments.
Comment at: docs/clang-tidy/checks/readability-redundant-preprocessor.rst:34
+
+ #ifdef FOO
+ #ifndef FOO // inner ifndef is considered redundant
The indendation for these examples (following as well) is not enough. Please
use 2
JonasToth added a comment.
>> I would be very interested why these false positives are created. Is there a
>> way to avoid them and maybe it makes sense to already add `FIXME` at the
>> possible places the check needs improvements.
>
> voidForLoopFalsePositive() and voidForLoopFalsePositive2()
ztamas added a comment.
In https://reviews.llvm.org/D53974#1294659, @JonasToth wrote:
> >> I would be very interested why these false positives are created. Is there
> >> a way to avoid them and maybe it makes sense to already add `FIXME` at the
> >> possible places the check needs improvements
brad added a comment.
Simon?
Repository:
rC Clang
https://reviews.llvm.org/D52418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
JonasToth added a comment.
> Yes, that's right, these are not full false positives, but the check's main
> focus is on those loops which are runtime dependent. If a loop's upper bound
> can be calculated in compile time then this loop should be caught by a
> compiler warning based on the actual
JonasToth added inline comments.
Comment at: test/clang-tidy/bugprone-too-small-loop-variable.cpp:138
+ bool cond = false;
+ for (short i = 0; i < (cond ? 0 : size); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower
type 'short' than iteration
JonasToth created this revision.
JonasToth added reviewers: aaron.ballman, hokein, alexfh, shuaiwang, lebedev.ri.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
JonasToth added a dependent revision: D45444: [clang-tidy] implement new check
for const-correctness.
This patch extends the
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, MTC.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, szepet, whisperity.
TL;DR: `CheckerOptInfo` feels very much out of place in
`CheckerRegistration.cpp
vmiklos updated this revision to Diff 173572.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readability/RedundantPreprocessorCheck.h
docs/Rele
vmiklos marked an inline comment as done.
vmiklos added inline comments.
Comment at: docs/clang-tidy/checks/readability-redundant-preprocessor.rst:34
+
+ #ifdef FOO
+ #ifndef FOO // inner ifndef is considered redundant
JonasToth wrote:
> The indendation for these
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts()
ztamas updated this revision to Diff 173573.
ztamas added a comment.
- Add requested test case
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/TooSmallLoopVariabl
ztamas added a comment.
> Agree. I think this check is good to go.
>
> I would commit this check tomorrow if that is ok with you.
Of course, It would be great if this check can get in, Thanks!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
vmiklos marked 2 inline comments as done.
vmiklos added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+
vmiklos updated this revision to Diff 173575.
vmiklos marked an inline comment as done.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readabilit
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts()
malaperle added a comment.
What is the goal for doing this without the AST? Is the goal to not have to
keep the AST and save memory?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52273
___
cfe-commits mailing list
cfe-commits@list
shuaiwang created this revision.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
a.sidorin, mgorny.
Herald added a reviewer: george.karpenkov.
This just copies ExprMutationAnalyzer to Tooling/Analysis with minor tweaks
around including path & namespaces.
2/3 will change exi
shuaiwang created this revision.
shuaiwang added reviewers: george.karpenkov, rsmith, dblaikie.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
a.sidorin, mgorny.
shuaiwang added a dependency: D54399: Move ExprMutationAnalyzer to
Tooling/Analysis (1/3).
Reference the new lo
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, whisperity.
This bugged me for a long time, so it's time to put an end to it:
`collectChecke
Szelethus updated this revision to Diff 173584.
Szelethus added a comment.
Also, remove `diags::note_suggest_disabling_all_checkers`.
https://reviews.llvm.org/D54401
Files:
include/clang/Basic/DiagnosticFrontendKinds.td
include/clang/StaticAnalyzer/Core/CheckerRegistry.h
include/clang/Sta
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
The list of matchers which can be used with a top matcher can be used in
other contexts than code completion. It can be output as data in
clang-tidy.
Repository:
rC Clang
https://
JonasToth added a comment.
Will you update the existing clang-tidy checks as well?
Repository:
rC Clang
https://reviews.llvm.org/D54399
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
The MatchingMatcher struct currently contains only a string, but it will
be expanded in the future.
Repository:
rC Clang
https://reviews.llvm.org/D54403
Files:
include/clang/AS
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D54404
Files:
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/ASTMatchers/Dynamic/RegistryTest.cpp
Index: unittests/ASTMatchers/D
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
These matchers are bindable. Recording this information will make it
possible to introspect the matchers which can be used inside another
matcher.
Repository:
rC Clang
https://rev
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
These correspond to the existing match() free functions.
Repository:
rC Clang
https://reviews.llvm.org/D54406
Files:
include/clang/ASTMatchers/ASTMatchFinder.h
unittests/ASTM
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
This is necessary so that when we wish to print the matchers for a
binding of type `CXXMeethodDecl`, but which was matched with a base
matcher such as `functionDecl()` we can inform th
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D54408
Files:
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/ASTMatchers/Dynamic/RegistryTest.cpp
Index: unittests/ASTMatchers/Dy
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:39
+static llvm::Optional
+GetScaleForFactory(llvm::StringRef FactoryName) {
+ static const std::unordered_map ScaleMap(
GetScaleForFactory -> getScaleForFactory, pe
steveire added a comment.
These commits are available on github if it's convenient to see it all together
there:
https://github.com/steveire/clang/commits/matcher-output
Here is context for the changes I'm making in case it is useful:
https://steveire.wordpress.com/2018/11/11/future-developmen
aaron.ballman added a comment.
Generally LGTM, but I leave it to someone more well-versed in ObjC to sign off
that this actually does everything those users would expect.
Comment at: docs/clang-tidy/checks/google-objc-function-naming.rst:12
+
+All function names should be in u
aaron.ballman added a comment.
Generally LGTM but this has no test coverage. What are your plans for how you
want to see this proceed? Do you intend to commit everything in one batch once
all of the reviews are accepted, or do piecemeal commits?
Comment at: lib/ASTMatchers/Dy
aaron.ballman added inline comments.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:604-605
+
+ std::vector AcceptedTypes;
+ AcceptedTypes.push_back(StaticType);
+
`std::vector AcceptedTypes{StaticType};` instead?
Comment at: lib/ASTMatcher
aaron.ballman added inline comments.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:604
+ static std::vector excludedMatchers{
+ "allOf",
`excludedMatchers` -> `ExcludedMatchers`
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:604
aaron.ballman added inline comments.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:70
+ void registerIfNodeMatcher(MatcherType,
+ internal::MatcherDescriptor *matchDescriptor,
+ StringRef) {}
Fix `matchD
aaron.ballman added inline comments.
Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:314
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, const
ast_type_traits::DynTypedNode &Node,
+ ASTContext &Context) {
80-col issue.
aaron.ballman added a reviewer: sbenza.
aaron.ballman added a subscriber: sbenza.
aaron.ballman added a comment.
Adding @sbenza in case he has opinions on this approach. I think it's
reasonable, but I also know that changes to the the AST matcher internals
sometimes have unintended side effects
erik.pilkington accepted this revision.
erik.pilkington added a comment.
LGTM too, with one small fix in test. Thanks for working on this!
Comment at: test/CodeGenCXX/attr-no-destroy-d54344.cpp:4
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
-D
sthibaul updated this revision to Diff 173599.
sthibaul added a comment.
In this version, the Driver introduces the "-hurd-" part
Repository:
rC Clang
https://reviews.llvm.org/D54379
Files:
lib/Basic/Targets.cpp
lib/Basic/Targets/OSTargets.h
lib/Driver/CMakeLists.txt
lib/Driver/Drive
sthibaul updated this revision to Diff 173601.
Repository:
rC Clang
https://reviews.llvm.org/D54379
Files:
lib/Basic/Targets.cpp
lib/Basic/Targets/OSTargets.h
lib/Driver/CMakeLists.txt
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/
kristina added a comment.
In https://reviews.llvm.org/D54344#1294942, @erik.pilkington wrote:
> LGTM too, with one small fix in test. Thanks for working on this!
Well, I figured since the assertion being tripped was (before investigation)
the only reliable way to notice the bug it makes sense
kristina updated this revision to Diff 173602.
kristina added a comment.
Revised to address comments.
Repository:
rC Clang
https://reviews.llvm.org/D54344
Files:
lib/CodeGen/CGDeclCXX.cpp
test/CodeGenCXX/attr-no-destroy-d54344.cpp
Index: test/CodeGenCXX/attr-no-destroy-d54344.cpp
=
kristina added a comment.
Though on the other hand it makes no sense to skip it with asserts off either,
that just clicked in my head, sorry.
Repository:
rC Clang
https://reviews.llvm.org/D54344
___
cfe-commits mailing list
cfe-commits@lists.llv
erik.pilkington added a comment.
In https://reviews.llvm.org/D54344#1294960, @kristina wrote:
> In https://reviews.llvm.org/D54344#1294942, @erik.pilkington wrote:
>
> > LGTM too, with one small fix in test. Thanks for working on this!
>
>
> Well, I figured since the assertion being tripped was (
sthibaul updated this revision to Diff 173604.
sthibaul added a comment.
Herald added a subscriber: srhines.
This includes version finding the gcc Hurd triplet (i[3456]-gnu) in the Gcc
detector.
Repository:
rC Clang
https://reviews.llvm.org/D54379
Files:
lib/Basic/Targets.cpp
lib/Basic/
Author: kristina
Date: Sun Nov 11 17:19:16 2018
New Revision: 346628
URL: http://llvm.org/viewvc/llvm-project?rev=346628&view=rev
Log:
[CodeGen][CXX]: Fix no_destroy CG bug under specific circumstances
Summary:
Class with no user-defined destructor that has an inherited member that has a
non-tri
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346628: [CodeGen][CXX]: Fix no_destroy CG bug under specific
circumstances (authored by kristina, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.o
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:84
+CheckFactories.registerCheck(
+"readability-redundant-preprocessor");
CheckFactories.registerCheck(
Please keep this list sorted alphabetically.
michaelwu updated this revision to Diff 173607.
michaelwu added a comment.
Thanks for the review! All requested changes have been made, and I also had to
fix a test due to https://reviews.llvm.org/D50214
https://reviews.llvm.org/D50318
Files:
include/clang/Basic/Attr.td
include/clang/Basic
Author: mwu
Date: Sun Nov 11 18:44:33 2018
New Revision: 346633
URL: http://llvm.org/viewvc/llvm-project?rev=346633&view=rev
Log:
Support Swift in platform availability attribute
Summary: This adds support for Swift platform availability attributes. It's
largely a port of the changes made to htt
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346633: Support Swift in platform availability attribute
(authored by mwu, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D50318
Files:
include/clang/Basic/Attr.td
include/clang/Bas
Quuxplusone updated this revision to Diff 173610.
Quuxplusone added a comment.
Rebased on master. @EricWF @ldionne ping please?
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
Files:
include/experimental/memory_resource
src/experimental/memory_resource.cpp
Index: src/experimenta
Quuxplusone updated this revision to Diff 173611.
Quuxplusone added a comment.
Rebased on master. @ericwf @ldionne ping please?
Repository:
rCXX libc++
https://reviews.llvm.org/D47111
Files:
include/experimental/memory_resource
src/experimental/memory_resource.cpp
test/libcxx/experime
Quuxplusone updated this revision to Diff 173612.
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
Files:
include/experimental/memory_resource
src/experimental/memory_resource.cpp
Index: src/experimental/memory_resource.cpp
==
Quuxplusone updated this revision to Diff 173613.
Quuxplusone added a comment.
Herald added a subscriber: libcxx-commits.
Rebased on master. @ericwf @ldionne ping please?
Repository:
rCXX libc++
https://reviews.llvm.org/D47358
Files:
include/experimental/memory_resource
src/experimental/
erik.pilkington created this revision.
erik.pilkington added a reviewer: rsmith.
Herald added a subscriber: dexonsmith.
Previously, we'd accept the attached test because we didn't substitute into
`void_t` when we really ought to of.
Fixes llvm.org/PR39623
Thanks!
Repository:
rC Clang
https
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346635: Fix compatibility with z3-4.8.1 (authored by
jankratochvil, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D54391?vs=173555&id=173618#
70 matches
Mail list logo
