Author: jonastoth
Date: Fri Nov 9 12:54:06 2018
New Revision: 346554
URL: http://llvm.org/viewvc/llvm-project?rev=346554&view=rev
Log:
[ASTMatchers] overload ignoringParens for Expr
Summary: This patch allows fixing PR39583.
Reviewers: aaron.ballman, sbenza, klimek
Reviewed By: aaron.ballman
JonasToth updated this revision to Diff 173423.
JonasToth added a comment.
- use ignoringParens instead
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54281
Files:
clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
test/clang-tidy/cppcoreguidelines-pro-bound
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346554: [ASTMatchers] overload ignoringParens for Expr
(authored by JonasToth, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D54307?vs=173422&id=173424#toc
Repository:
rC Clang
h
Author: jonastoth
Date: Fri Nov 9 12:57:28 2018
New Revision: 346555
URL: http://llvm.org/viewvc/llvm-project?rev=346555&view=rev
Log:
[clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in
pro-bounds-array-to-pointer-decay
Summary:
The fix to the issue that `const char* p = ("foo
JonasToth updated this revision to Diff 173425.
JonasToth added a comment.
- fix comment
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54281
Files:
clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-poin
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346555: [clang-tidy] fix PR39583 - ignoring ParenCast for
string-literals in pro-bounds… (authored by JonasToth, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://rev
aaron.ballman added inline comments.
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:55
+ // to use.
+ if (Decl->getStorageClass() != SC_Static) {
+return FixItHint();
Elide braces.
Comment at: clang-tidy/google/FunctionNamingCheck.
vmiklos created this revision.
vmiklos added reviewers: JonasToth, alexfh.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai.
Finds potentially redundant preprocessor usage.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54349
Files:
clang-tidy/reada
Author: adrian
Date: Fri Nov 9 13:17:38 2018
New Revision: 346556
URL: http://llvm.org/viewvc/llvm-project?rev=346556&view=rev
Log:
Revert "Revert rL346454: Fix a use-after-free introduced by r344915."
This un-reverts commit 346454 with a relaxed CHECK for Windows.
Added:
cfe/trunk/test/Cod
vmiklos added a comment.
I've used this check originally on LibreOffice (core, online) code and it found
a handful of not necessary ifdef / ifndef lines. It seemed it's simple and
generic enough that it would make sense to upstream this.
Repository:
rCTE Clang Tools Extra
https://reviews.ll
kristina added a comment.
In https://reviews.llvm.org/D54344#1293468, @erik.pilkington wrote:
> Have you tried running creduce on the preprocessed source? We should really
> have a real reproducer for this, otherwise its really hard to tell what the
> underlying problem is.
Unable to build it
lebedev.ri added a comment.
No one will know for sure what "pp" in "readability-redundant-pp" means.
I'd highly recommend to fully spell it out.
Also, i'd like to see some analysis of the false-positives.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54349
_
JonasToth added a comment.
Hi vmiklos,
thank you for working on this patch!
I added a few comments.
Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:83
"readability-misplaced-array-index");
+CheckFactories.registerCheck("readability-redundant-pp");
aaron.ballman added a comment.
In https://reviews.llvm.org/D54344#1293468, @erik.pilkington wrote:
> Have you tried running creduce on the preprocessed source? We should really
> have a real reproducer for this, otherwise its really hard to tell what the
> underlying problem is.
I'm going to
vmiklos added a comment.
In https://reviews.llvm.org/D54349#1293622, @lebedev.ri wrote:
> No one will know for sure what "pp" in "readability-redundant-pp" means.
> I'd highly recommend to fully spell it out.
Will do.
> Also, i'd like to see some analysis of the false-positives.
Things I con
Eugene.Zelenko added inline comments.
Comment at: docs/ReleaseNotes.rst:73
+
+ Finds potentially redundant preprocessor usage.
+
preprocessor directives? Same in documentation.
Comment at: docs/clang-tidy/checks/readability-redundant-pp.rst:8
void created this revision.
void added a reviewer: rsmith.
Herald added subscribers: cfe-commits, kristina.
Herald added a reviewer: shafik.
A __builtin_constant_p may end up with a constant after inlining. Use
the is.constant intrinsic if it's a variable that's in a context where
it may resolve t
void created this revision.
void added reviewers: rsmith, shafik.
Herald added a subscriber: jfb.
This cleans up the code somewhat and allows us conditionally to act on
different types of nodes depending on their context. E.g., if we're
checking for an ICE in a constant context.
Repository:
rC
void updated this revision to Diff 173445.
void added a comment.
Herald added a subscriber: jfb.
Adding ConstantExpr visitor.
Repository:
rC Clang
https://reviews.llvm.org/D54355
Files:
include/clang/AST/Expr.h
lib/AST/ASTImporter.cpp
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/C
void added a comment.
This has https://reviews.llvm.org/D54356 integrated into it.
https://reviews.llvm.org/D54356 should be reviewed and submitted first, even
though it's out of order.
Repository:
rC Clang
https://reviews.llvm.org/D54355
___
c
NoQ added a comment.
Thx!! Tiny nits.
Comment at: www/analyzer/checker_dev_manual.html:720
+User facing documentation is important for adoption! Make sure the check
list updated
+at the homepage of the analyzer. Also ensure that the description is good
quality in
+Che
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Thanks, this is really wonderful.
Comment at: test/Analysis/analyzer-config.cpp:1
// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null
-analyzer-checker=core,
NoQ accepted this revision.
NoQ added a comment.
Thx!! Burn it.
> tblgen files look awesome. I get that this isn't a very strong point, but
> it's a pretty sight compared to a .def file.
I agree that it's aesthetically satisfying, but it is (1) inconvenient to write
because that's the whole ne
Szelethus marked an inline comment as done.
Szelethus added a comment.
Thanks! This patch was the last for a while directly related to non-checker
config options, I'm currently stuck on them as I unearthed countless bugs that
I have to fix beforehand. (Did you know that clang unit tests do not r
NoQ added a comment.
In https://reviews.llvm.org/D53692#1293778, @Szelethus wrote:
> Did you know that clang unit tests do not run with check-clang-analysis?
Well, *some* unit tests definitely do run under check-clang-analysis.
Repository:
rC Clang
https://reviews.llvm.org/D53692
__
ztamas added a comment.
In https://reviews.llvm.org/D53974#1293268, @JonasToth wrote:
> LGTM.
> Did you run this check in its final form against a bigger project? These
> results would be interesting.
I'll run it on LibreOffice code again and we'll see.
> Do you have commit access?
Commit a
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346566: Fix ClangFormat issue of recognizing ObjC subscript
as C++ attributes when… (authored by Wizard, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llv
Author: wizard
Date: Fri Nov 9 15:19:14 2018
New Revision: 346566
URL: http://llvm.org/viewvc/llvm-project?rev=346566&view=rev
Log:
Fix ClangFormat issue of recognizing ObjC subscript as C++ attributes when
message target is a result of a C-style method.
Summary:
The issue is that for array sub
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346566: Fix ClangFormat issue of recognizing ObjC subscript
as C++ attributes when… (authored by Wizard, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D54288?vs=173258&id=173457#toc
Szelethus added a comment.
> I agree that it's aesthetically satisfying, but it is (1) inconvenient to
> write because that's the whole new syntax you need to memorize, i.e. all
> those <> and semicolons and (2) not cooperating when i want to look up
> checker name by source file (have to scan
kristina added a comment.
In https://reviews.llvm.org/D54344#1293643, @aaron.ballman wrote:
> In https://reviews.llvm.org/D54344#1293468, @erik.pilkington wrote:
>
> > Have you tried running creduce on the preprocessed source? We should really
> > have a real reproducer for this, otherwise its r
erik.pilkington added inline comments.
Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203
+// If the header in the module map refers to a symlink, Header.Entry
+// refers to the actual file. The callback should be notified of both.
+if (!FullPathAsWritten.empty() &&
+
pcc added inline comments.
Comment at: docs/LTOVisibility.rst:9
unit's *LTO unit* is the subset of the linkage unit that is linked together
-using link-time optimization; in the case where LTO is not being used, the
-linkage unit's LTO unit is empty. Each linkage unit has only a
vmiklos updated this revision to Diff 173465.
vmiklos retitled this revision from "[clang-tidy] new check
'readability-redundant-pp'" to "[clang-tidy] new check
'readability-redundant-preprocessor'".
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/rea
vmiklos added a comment.
> I think that name is not very descriptive for the user of clang-tidy. `pp`
> should be `preprocessor` or some other constellation that makes it very clear
> its about the preprocessor.
Done, renamed to readability-redundant-preprocessor.
> you are in namespace `clang
vmiklos updated this revision to Diff 173468.
vmiklos edited the summary of this revision.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readabi
vmiklos added a comment.
> preprocessor directives? Same in documentation.
Done.
https://reviews.llvm.org/D54349
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaron.ballman added a comment.
What do you think about code like:
#if FOO == 4
#if FOO == 4
#endif
#endif
#if defined(FOO)
#if defined(FOO)
#endif
#endif
#if !defined(FOO)
#if !defined(FOO)
#endif
#endif
#if defined(FOO)
#if !defined(FOO)
#endif
#endif
NoQ added a comment.
Thanks! I don't have much to add to the review, but i very much approve this
check.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54222
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm
NoQ updated this revision to Diff 173487.
NoQ added a comment.
Add an interesting test for the `MisusedMovedObject` checker that demonstrates
one more potential source of false positives caused by the zombie symbol
problem. In this test there are, well, //no symbols//. Therefore, there are no
d
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, baloghadamsoftware.
Since https://reviews.llvm.org/D18860 addresses the problem that some re
NoQ added a comment.
The manual region cleanup is removed from `MisusedMovedObjectChecker` in
https://reviews.llvm.org/D54372.
> Another thing, since we're directly testing the functionality of
> `SymbolReaper`, maybe it'd be worth to also include unit tests.
Sounds like a great idea, and/or i
dblaikie added a comment.
OK - thanks for that.
I'm going to make an executive decision on the naming. Let's go with
-gsplit-dwarf[=single] (or explicitly -gsplit-dwarf=split, which is the default
value when -gsplit-dwarf is specified). Saves adding a new name/flag, avoids
the use of the word
vsapsai added inline comments.
Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203
+// If the header in the module map refers to a symlink, Header.Entry
+// refers to the actual file. The callback should be notified of both.
+if (!FullPathAsWritten.empty() &&
+!
kristina added a comment.
This seems to be a peculiar side effect of weird combinations of previous uses
of attributes `no_destroy`, `used`, and `section`, as well as complex macros
used to create metaclass-like systems (through the use of said attributes). It
seems that there is a serious bug
kristina created this revision.
kristina added a reviewer: clang.
Herald added a subscriber: cfe-commits.
Noticed while working with that area of code, NFC.
Repository:
rC Clang
https://reviews.llvm.org/D54373
Files:
lib/CodeGen/CGDeclCXX.cpp
Index: lib/CodeGen/CGDeclCXX.cpp
Author: kristina
Date: Fri Nov 9 23:53:47 2018
New Revision: 346582
URL: http://llvm.org/viewvc/llvm-project?rev=346582&view=rev
Log:
Correct naming conventions and 80 col rule violation in CGDeclCXX.cpp. NFC.
Differential Revision: https://reviews.llvm.org/D54373
Modified:
cfe/trunk/lib/C
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346582: Correct naming conventions and 80 col rule violation
in CGDeclCXX.cpp. NFC. (authored by kristina, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://revie
101 - 148 of 148 matches
Mail list logo