alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good with a few style comments.
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:82
@@ +81,3 @@
+
+ // If we see int/short/struct etc just assume this is a variabl
alexfh added a comment.
In http://reviews.llvm.org/D20689#443266, @varjujan wrote:
> Yes, I did. The results from running the checker on LLVM are in the attached
> file. Sadly, I could'nt find any real mistakes but as I wrote in the summary,
> false positives can still indicate bad naming conve
alexfh resigned from this revision.
alexfh removed a reviewer: alexfh.
alexfh added a comment.
Cleaning up my Phab dashboard. If http://reviews.llvm.org/D20428 doesn't happen
and we'll have to return to this implementation, please re-add me to the
reviewers.
http://reviews.llvm.org/D18575
_
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:31
@@ +30,3 @@
+MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus) return;
+
clang-format
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Removing from my dashboard until http://reviews.llvm.org/D20428 is submitted.
http://reviews.llvm.org/D20693
___
cfe-commits mailing li
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
> -Wdeprecated-increment-bool does it. But what I see from SemaExpr.cpp in
> CheckIncrementDecrementOperand, it doesn't have any fixits.
If an automated fix for this issue makes sen
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:37
@@ +36,3 @@
+
+It turns out that the common bug is to have function returning only bools but
having int as retur
alexfh added a comment.
Most of the fixits are still useless, but I didn't notice any false positives
(i.e. all warnings would be useful to some degree). So I suggest disabling the
fixes completely at least for now.
Comment at: include/llvm-c/Core.h:604
@@ -603,3 +603,3 @@
*
alexfh added a comment.
Looks like a useful check to have. I'm not sure though, that it has anything to
do with "modernize". I'd suggest adding a new "bugprone" module (should be
added by http://reviews.llvm.org/D18821, hopefully soon) and moving the check
there.
Comment at:
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG with one comment.
Comment at: test/clang-tidy/misc-definitions-in-headers-cxx11.hpp:6
@@ +5,3 @@
+
+int f() = delete;
+// CHECK-NOT: [misc-definitions-in-headers]
-
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good. Thank you for fixing the copy-paste failures!
Do you need me to commit the patch for you?
Comment at: clang-tidy/tool/clang-tidy-diff.py:42
@@ -41,3 +41,3 @@
Author: alexfh
Date: Tue Jun 7 04:11:19 2016
New Revision: 271992
URL: http://llvm.org/viewvc/llvm-project?rev=271992&view=rev
Log:
[clang-tidy] readability-identifier-naming - Support for Type Aliases
Summary: Added support for Type Alias declarations.
Reviewers: alexfh
Subscribers: cfe-commi
This revision was automatically updated to reflect the committed changes.
Closed by commit rL271992: [clang-tidy] readability-identifier-naming - Support
for Type Aliases (authored by alexfh).
Changed prior to commit:
http://reviews.llvm.org/D20856?vs=59189&id=59847#toc
Repository:
rL LLVM
Author: alexfh
Date: Wed Jun 8 09:27:43 2016
New Revision: 272144
URL: http://llvm.org/viewvc/llvm-project?rev=272144&view=rev
Log:
[clang-tidy] correct clang-tidy-diff.py help message
Summary:
Looks like the original code was copied from clang-format-diff.py.
Update help message to make it clan
This revision was automatically updated to reflect the committed changes.
Closed by commit rL272144: [clang-tidy] correct clang-tidy-diff.py help message
(authored by alexfh).
Changed prior to commit:
http://reviews.llvm.org/D21050?vs=59813&id=60035#toc
Repository:
rL LLVM
http://reviews.ll
alexfh added inline comments.
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:142
@@ +141,3 @@
+SourceManager &SM(PP->getSourceManager());
+if (!SM.isInMainFile(MacroNameTok.getLocation()))
+ return;
I'm not sure the check currently refus
Author: alexfh
Date: Wed Jun 8 10:27:46 2016
New Revision: 272155
URL: http://llvm.org/viewvc/llvm-project?rev=272155&view=rev
Log:
[clang-tidy] misc-argument-comment: don't bail out when an argument is a macro
expansion (e.g. NULL).
Add CHECK-FIX tests.
Modified:
clang-tools-extra/trunk/c
alexfh added a comment.
Thank you for working on this! A few more comments in addition to what Aaron
has written.
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:22
@@ +21,3 @@
+
+void ReplaceCallWithArg(const CallExpr *TheCallExpr, DiagnosticBuilder *Diag,
+
alexfh added a comment.
Does this just fix the tests broken with http://reviews.llvm.org/D20498 or have
you looked at other checks that might be broken by
http://reviews.llvm.org/D20498, but don't have relevant tests? In the latter
case we'd certainly need your help figuring out where further p
alexfh added a comment.
In http://reviews.llvm.org/D21243#455999, @alexfh wrote:
> Does this just fix the tests broken with http://reviews.llvm.org/D20498 or
> have you looked at other checks that might be broken by
> http://reviews.llvm.org/D20498, but don't have relevant tests? In the latter
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:41
@@ +40,3 @@
+
+ const auto PlusOperatorMatcher =
+ cxxOperatorCallExpr(
s/Matcher//
=
alexfh accepted this revision.
alexfh added a comment.
LG. Thanks!
http://reviews.llvm.org/D21223
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alexfh added a comment.
BTW, I'll commit the patch for you.
http://reviews.llvm.org/D21223
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG. I'll commit the patch for you.
http://reviews.llvm.org/D21366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
Author: alexfh
Date: Thu Jun 16 09:32:54 2016
New Revision: 272897
URL: http://llvm.org/viewvc/llvm-project?rev=272897&view=rev
Log:
[clang-tidy] misc-move-const-arg: Fix typos
Reviewers: alexfh
Subscribers: cfe-commits
Patch by Martin Boehme!
Differential Revision: http://reviews.llvm.org/D21
This revision was automatically updated to reflect the committed changes.
Closed by commit rL272896: [clang-tidy] misc-move-const-arg: Detect if result
of std::move() is being… (authored by alexfh).
Changed prior to commit:
http://reviews.llvm.org/D21223?vs=60686&id=60974#toc
Repository:
rL
This revision was automatically updated to reflect the committed changes.
Closed by commit rL272897: [clang-tidy] misc-move-const-arg: Fix typos
(authored by alexfh).
Changed prior to commit:
http://reviews.llvm.org/D21366?vs=60802&id=60975#toc
Repository:
rL LLVM
http://reviews.llvm.org/D2
Author: alexfh
Date: Thu Jun 16 09:32:41 2016
New Revision: 272896
URL: http://llvm.org/viewvc/llvm-project?rev=272896&view=rev
Log:
[clang-tidy] misc-move-const-arg: Detect if result of std::move() is being
passed as a const ref argument
Summary:
Conceptually, this is very close to the existing
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good with a couple of nits.
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:808
@@ +807,3 @@
+ addUsage(NamingCheckFailures, ID, Range);
+ return;
+}
alexfh added a comment.
The check seems reasonable, I'm surprised there's no warning in Clang that
catches index[array] syntax ;)
A few comments re: implementation.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:43
@@ +42,3 @@
+
+static StringRef getAsString(c
alexfh added a comment.
In http://reviews.llvm.org/D21020#460734, @JamesReynolds wrote:
> Thanks! All fixed now. Could you land this for me please?
Sure, will do. Thank you for working on this!
http://reviews.llvm.org/D21020
___
cfe-commits maili
Author: alexfh
Date: Fri Jun 17 04:25:24 2016
New Revision: 272993
URL: http://llvm.org/viewvc/llvm-project?rev=272993&view=rev
Log:
[clang-tidy] readability-identifier-naming - Support for Macros
Summary:
Added support for macro definitions.
--
1. Added a pre-processor callback to catch macro d
This revision was automatically updated to reflect the committed changes.
Closed by commit rL272993: [clang-tidy] readability-identifier-naming - Support
for Macros (authored by alexfh).
Changed prior to commit:
http://reviews.llvm.org/D21020?vs=61071&id=61074#toc
Repository:
rL LLVM
http:/
Author: alexfh
Date: Fri Jun 17 06:43:33 2016
New Revision: 272994
URL: http://llvm.org/viewvc/llvm-project?rev=272994&view=rev
Log:
[clang-tidy] Fix doxygen errors. NFC.
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.h
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeader
Author: alexfh
Date: Fri Jun 17 07:01:15 2016
New Revision: 272995
URL: http://llvm.org/viewvc/llvm-project?rev=272995&view=rev
Log:
[clang-tidy] More doc fixes. NFC.
Modified:
clang-tools-extra/trunk/clang-tidy/utils/IncludeSorter.h
clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h
alexfh added a comment.
A few nits. Otherwise looks good, if Sam has no objections.
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:34
@@ -29,1 +33,3 @@
+template bool isSetDifferenceEmpty(const S &S1, const S &S2) {
+ for (const auto &E : S1)
-
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Apart from the other reviewers' comments, I have a few concerns:
1. The "performance-returning-type" name is missing an important part of
information. Maybe "performance-return-value
alexfh added a comment.
Removing from my dashboard until http://reviews.llvm.org/D20428 is submitted.
http://reviews.llvm.org/D20693
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Removing from my dashboard until http://reviews.llvm.org/D20428 is submitted.
Repository:
rL LLVM
http://reviews.llvm.org/D19201
___
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Yes, you're probably not the right person to fix our test coverage. Thanks for
fixing the stuff that broke and I hope we'll figure out what else needs to be
fixed in a finite time after the ch
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
http://reviews.llvm.org/D21470
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
alexfh added a comment.
A few nits.
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:135
@@ +134,3 @@
+ llvm::APSInt ValueLHS_plus1;
+ if (((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
+ (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
How about r
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG with a couple of nits.
Comment at: test/clang-tidy/boost-use-to-string.cpp:154
@@ +153,3 @@
+ float floating;
+ Fields* wierd;
+ const int &getConstInteger() const {retu
alexfh accepted this revision.
alexfh added a comment.
LG
http://reviews.llvm.org/D21747
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alexfh added inline comments.
Comment at: test/clang-tidy/boost-use-to-string.cpp:154
@@ +153,3 @@
+ float floating;
+ Fields* wierd;
+ const int &getConstInteger() const {return integer;}
alexfh wrote:
> "wierd" is weird ;)
I should have been more clear in the
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
A couple of nits. Otherwise looks good. Thanks!
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:232
@@ +231,3 @@
+
+static bool rangeSubsumeRange(BinaryOperatorKind O
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG after addressing the comment. Thanks!
Comment at: test/clang-tidy/performance-unnecessary-value-param.cpp:234
@@ +233,3 @@
+};
+
+struct PositiveNonVirualMethod {
-
alexfh added a comment.
IIUC, Stroustrup style mandates Single_capital_letter_to_start_a_type_name
(http://www.stroustrup.com/Programming/PPP-style.pdf). So both styles you're
adding in this patch should not be named after Stroustrup. Also, do these come
from real-life projects/requirements or
alexfh added inline comments.
Comment at: clang-tidy/google/GlobalNamesInHeadersCheck.cpp:49
@@ -48,1 +48,3 @@
+namespace {
+std::string getIncludeFromMessage(SourceLocation DeclLoc,
Static functions are preferred in LLVM.
http://llvm.org/docs/CodingStandards.h
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
http://reviews.llvm.org/D21833
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
alexfh added a comment.
Please re-generate the diff with full-context
(http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).
A bunch of style comments.
Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:86
@@ +85,3 @@
+
+void MpiTypeMismatchCheck:
alexfh added inline comments.
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:50
@@ +49,3 @@
+ hasOverloadedOperatorName("="), hasDescendant(BasicStringPlusOperator),
+ allOf(hasArgument(
+0, allOf(declRefExpr(BasicStringType),
alexfh added inline comments.
Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:99
@@ -88,3 +98,3 @@
//
// 1. Insert std prefix for every such symbol occurance.
// 2. Insert `using namespace std;` to the beginning of TU.
nit: "occurrence"
=
alexfh added a comment.
In http://reviews.llvm.org/D21472#473813, @JamesReynolds wrote:
> Ah, I took this from a single example in the CPP core guidelines - that PDF
> is indeed a very different style. An idea we toyed with was "UpperSeparated"
> and "UpperSeparatedBack"? Would that work?
You
alexfh added inline comments.
Comment at: docs/clang-tidy/checks/modernize-deprecated-headers.rst:39
@@ +38,3 @@
+* `` -> ``
+* `` -> `` // deprecated since C++11
+* `` -> ``
alexfh wrote:
> Not sure if "deprecated" is the right word here. The wording seems to
alexfh added a comment.
Gergely, it seems that the last diff is missing
clang-tidy/readability/MisleadingIndentationCheck.cpp. A few more comments
below.
Comment at: docs/clang-tidy/checks/readability-misleading-indentation.rst:13
@@ +12,3 @@
+
+The way to avoid dangling else
alexfh added a comment.
Thank you for fixing the bug! One comment below.
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:525
@@ +524,3 @@
+ DeclarationType = DeclarationType.getNonReferenceType();
+if (Descriptor.ElemType.isNull() || DeclarationType.isNull() ||
+
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
http://reviews.llvm.org/D22046
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
alexfh added inline comments.
Comment at: clang-rename/tool/clang-rename.py:12
@@ +11,3 @@
+
+map ,cf :pyf /clang-include-fixer.py
+
ioeric wrote:
> Maybe a different key binding so that it doesn't conflict with
> include-fixer's suggested key binding? `,cr`
alexfh added inline comments.
Comment at: clang-rename/tool/ClangRename.cpp:39
@@ -38,3 +38,2 @@
#include "llvm/Support/Host.h"
-#include
#include
bkramer wrote:
> omtcyf0 wrote:
> > bkramer wrote:
> > > This looks unrelated.
> > Right, but I'm not sure one-li
alexfh accepted this revision.
alexfh added a comment.
LG
http://reviews.llvm.org/D22087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alexfh added inline comments.
Comment at: clang-rename/tool/clang-rename.py:17
@@ +16,3 @@
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the
+cursor points to a valid sym
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:525
@@ +524,3 @@
+ DeclarationType = DeclarationType.getNonReferenceType();
+if (Descriptor.ElemType.isNull() || DeclarationType
alexfh added a comment.
Thanks for the new awesome check!
Please run the check on LLVM and include your analysis of the results in the
patch description. Another couple of comments below.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:93
@@ +92,3 @@
+ continue;
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:147
@@ +146,3 @@
+const Type *MpiTypeMismatchCheck::argumentType(const CallExpr *const CE,
+
alexfh added a comment.
Sorry for the delay. Your patch was lost in my inbox. Feel free to ping me
earlier.
Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:37
@@ +36,3 @@
+
+It turns out that the common bug is to have function returning only bools but
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Sorry for the delay. Feel free to ping me earlier.
Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:38
@@ +37,3 @@
+ Finder->addMatcher(
+ cxxCon
This revision was automatically updated to reflect the committed changes.
Closed by commit rL274835: [ASTMatchers] Add missing forEachArgumentWithParam()
to code sample (authored by alexfh).
Changed prior to commit:
http://reviews.llvm.org/D21799?vs=62103&id=63196#toc
Repository:
rL LLVM
ht
This revision was automatically updated to reflect the committed changes.
Closed by commit rL274834: CFGBuilder: Fix crash when visiting a range-based
for over a dependent type (authored by alexfh).
Changed prior to commit:
http://reviews.llvm.org/D21895?vs=62374&id=63194#toc
Repository:
rL
Author: alexfh
Date: Fri Jul 8 05:51:00 2016
New Revision: 274835
URL: http://llvm.org/viewvc/llvm-project?rev=274835&view=rev
Log:
[ASTMatchers] Add missing forEachArgumentWithParam() to code sample
Reviewers: klimek
Subscribers: cfe-commits, klimek
Patch by Martin Boehme!
Differential Revis
Author: alexfh
Date: Fri Jul 8 05:50:51 2016
New Revision: 274834
URL: http://llvm.org/viewvc/llvm-project?rev=274834&view=rev
Log:
CFGBuilder: Fix crash when visiting a range-based for over a dependent type
Summary:
CFG generation is expected to fail in this case, but it should not crash.
Also
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
http://reviews.llvm.org/D22154
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG. Thanks!
Is it related to https://llvm.org/bugs/show_bug.cgi?id=28481 ?
http://reviews.llvm.org/D22188
___
cfe-commits mailing list
cfe-commi
alexfh added inline comments.
Comment at: clang-rename/USRLocFinder.cpp:63
@@ -62,3 +62,3 @@
const ASTContext &Context = ConstructorDecl->getASTContext();
-for (clang::CXXConstructorDecl::init_const_iterator it =
ConstructorDecl->init_begin(); it != ConstructorDecl->init
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG with one comment.
Comment at: clang-rename/USRLocFinder.cpp:73-76
@@ -73,3 +72,6 @@
SourceLocation Location = Initializer->getSourceLocation();
- String
alexfh added a comment.
In http://reviews.llvm.org/D22091#483834, @omtcyf0 wrote:
> Thanks, @alexfh!
>
> Can you please land it?
It's not easy to do from the phone ;) Please ask someone else (Ben?).
http://reviews.llvm.org/D22091
___
cfe-commits
alexfh added a comment.
In http://reviews.llvm.org/D17586#363466, @Eugene.Zelenko wrote:
> Sorry for arriving late to party :-)
>
> Thank you for great check! I found a quite lot of such problems in my code
> base, but I'd like to make suggestion for further improvements: PR26756.
The full lin
Author: alexfh
Date: Sat Feb 27 08:02:08 2016
New Revision: 262138
URL: http://llvm.org/viewvc/llvm-project?rev=262138&view=rev
Log:
Workaround doxygen bug https://bugzilla.gnome.org/show_bug.cgi?id=506243
Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG. Thanks!
Comment at: test/clang-tidy/modernize-pass-by-value-header.cpp:1
@@ +1,2 @@
+// RUN: cp %S/Inputs/modernize-pass-by-value/header.h %T/pass-by-value-header.h
+// RU
Thanks for the fix! When did it start failing? The last change in this file
was quite a while ago. Did it start breaking because of a change in a
header?
On Mon, Feb 29, 2016 at 10:17 PM, Hans Wennborg via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: hans
> Date: Mon Feb 29 15:17:39
+cfe-commits for wider audience.
Hi Piotr,
On Tue, Mar 1, 2016 at 2:36 PM, Piotr Padlewski
wrote:
> Hi Alexey,
>
I am currently writing new check
> https://llvm.org/bugs/show_bug.cgi?id=26763
>
Awesome! Sorry I didn't have time yet to comment on the issues you filed,
though I have a couple of
Answered on the review thread. The problem is with BuiltinTypeLoc not
storing the location for the end token of builtin type names consisting of
multiple tokens. This can be worked around by re-lexing a fragment of the
code for now.
Richard, does it make sense to store the end location in BuiltinT
alexfh added inline comments.
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:25-32
@@ +24,10 @@
+std::string paramNameOrIndex(StringRef Name, size_t Index) {
+ std::string Output;
+ llvm::raw_string_ostream Stream(Output);
+ if (!Name.empty()) {
+Stream <
alexfh added a comment.
As David noted, there are other similar classes like `llvm::StringRef`,
`llvm::ArraryRef`, `gsl::span`, `StringPiece` (in various Google projects, e.g.
https://github.com/google/re2/blob/master/re2/stringpiece.h) and numerous other
in different code bases. I think, we sh
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good. Thanks!
http://reviews.llvm.org/D17488
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
alexfh added inline comments.
Comment at: clang-tidy/utils/FixItHintUtils.cpp:2
@@ +1,3 @@
+//===--- FixItHintUtils.cpp -
+// clang-tidy---===//
+//
Please fix the line break.
http://reviews.llvm.org/D17488
Kirill, FYI.
On Thu, Feb 25, 2016 at 3:21 AM, Richard Smith
wrote:
> On Wed, Feb 24, 2016 at 5:36 AM, Alexander Kornienko via cfe-commits
> wrote:
> > Author: alexfh
> > Date: Wed Feb 24 07:36:34 2016
> > New Revision: 261738
> >
> > URL: http://llvm.org/vie
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good! Thank you for finding and fixing this!
Comment at: clang-tidy/utils/IncludeInserter.cpp:23
@@ -21,3 +22,3 @@
void InclusionDirective(SourceLocation HashLocation
Author: alexfh
Date: Thu Mar 3 04:42:46 2016
New Revision: 262603
URL: http://llvm.org/viewvc/llvm-project?rev=262603&view=rev
Log:
[docs] Updated doxygen files to work well with doxygen 1.8.11
Doxygen 1.8.11 doesn't seem to like files with ".intro" extension by default.
Removed:
cfe/trunk/
Author: alexfh
Date: Thu Mar 3 04:44:10 2016
New Revision: 262604
URL: http://llvm.org/viewvc/llvm-project?rev=262604&view=rev
Log:
[docs] Add missing file
Added:
cfe/trunk/docs/doxygen-mainpage.dox
Added: cfe/trunk/docs/doxygen-mainpage.dox
URL:
http://llvm.org/viewvc/llvm-project/cfe/tru
Author: alexfh
Date: Thu Mar 3 04:45:59 2016
New Revision: 262605
URL: http://llvm.org/viewvc/llvm-project?rev=262605&view=rev
Log:
[docs] Fix docs to work with doxygen 1.8.11
Added:
clang-tools-extra/trunk/docs/doxygen-mainpage.dox
Removed:
clang-tools-extra/trunk/docs/doxygen.intro
Mod
On Thu, Mar 3, 2016 at 2:08 PM, Gabor Horvath via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: xazax
> Date: Thu Mar 3 07:08:11 2016
> New Revision: 262615
>
> URL: http://llvm.org/viewvc/llvm-project?rev=262615&view=rev
> Log:
> [clang-tidy] Do not emit warnings from misc-suspiciou
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
Repository:
rL LLVM
http://reviews.llvm.org/D17849
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
Thank you!
On Thu, Mar 3, 2016 at 2:48 PM, Gábor Horváth wrote:
> Thank you for the suggestions. This should be addressed in r262618.
>
> On 3 March 2016 at 14:42, Alexander Kornienko wrote:
>
>>
>>
>> On Thu, Mar 3, 2016 at 2:08 PM, Gabor Horvath via cfe-commits <
>> cfe-commits@lists.llvm.org
Author: alexfh
Date: Sat Mar 5 22:05:59 2016
New Revision: 262787
URL: http://llvm.org/viewvc/llvm-project?rev=262787&view=rev
Log:
[docs] Clean up doxygen comments a bit.
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.h
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h
URL:
alexfh added inline comments.
Comment at: clang-tidy/misc/DanglingHandleCheck.cpp:24
@@ +23,3 @@
+
+std::vector parseClasses(StringRef Option) {
+ SmallVector Classes;
A very similar code has been added recently to
clang-tidy/utils/HeaderFileExtensionsUtils.*. M
alexfh added inline comments.
Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:20
@@ +19,3 @@
+namespace {
+bool isOverrideMethod(const FunctionDecl *Function) {
+ if (const auto *MD = dyn_cast(Function))
There's a matcher for this: `isOverride`.
Repositor
alexfh added inline comments.
Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:80
@@ +79,3 @@
+ UsedByRef() ||
+ !ast_matchers::match(cxxMethodDecl(isOverride()), *Function,
+ *Result.Context)
I meant, you can use it inside
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG. Thank you for one more awesome check!
Comment at: clang-tidy/misc/DanglingHandleCheck.cpp:25
@@ +24,3 @@
+std::vector parseClasses(StringRef Option) {
+ SmallVector Class
alexfh added a comment.
Sorry for leaving this without attention for a while.
I'm somewhat concerned about this change. It's adding a certain level of
complexity, but doesn't cover some less trivial cases like handling of multiple
headers. Can you take a look at existing tests and say how many
1401 - 1500 of 1905 matches
Mail list logo