r346554 - [ASTMatchers] overload ignoringParens for Expr

2018-11-09 Thread Jonas Toth via cfe-commits
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

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits
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

[PATCH] D54307: [ASTMatchers] overload ignoringParens for Expr

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits
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

[clang-tools-extra] r346555 - [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

2018-11-09 Thread Jonas Toth via cfe-commits
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

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits
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

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits
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

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits
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.

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits
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

r346556 - Revert "Revert rL346454: Fix a use-after-free introduced by r344915."

2018-11-09 Thread Adrian Prantl via cfe-commits
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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits
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

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits
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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Roman Lebedev via Phabricator via cfe-commits
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 _

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits
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");

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits
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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits
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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Eugene Zelenko via Phabricator via cfe-commits
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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

2018-11-09 Thread Bill Wendling via Phabricator via cfe-commits
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

[PATCH] D54356: Convert CheckICE into a statment visitor

2018-11-09 Thread Bill Wendling via Phabricator via cfe-commits
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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

2018-11-09 Thread Bill Wendling via Phabricator via cfe-commits
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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

2018-11-09 Thread Bill Wendling via Phabricator via cfe-commits
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

[PATCH] D52984: [analyzer] Checker reviewer's checklist

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits
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

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits
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,

[PATCH] D53995: [analyzer] Drastically simplify the tblgen files used for checkers

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits
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

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

2018-11-09 Thread Umann KristĂłf via Phabricator via cfe-commits
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

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits
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 __

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

2018-11-09 Thread Tamás Zolnai via Phabricator via cfe-commits
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

[PATCH] D54288: Fix ClangFormat issue of recognizing ObjC subscript as C++ attributes when message target is a result of a C-style method.

2018-11-09 Thread Yan Zhang via Phabricator via cfe-commits
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

r346566 - Fix ClangFormat issue of recognizing ObjC subscript as C++ attributes when message target is a result of a C-style method.

2018-11-09 Thread Yan Zhang via cfe-commits
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

[PATCH] D54288: Fix ClangFormat issue of recognizing ObjC subscript as C++ attributes when message target is a result of a C-style method.

2018-11-09 Thread Yan Zhang via Phabricator via cfe-commits
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

[PATCH] D53995: [analyzer] Drastically simplify the tblgen files used for checkers

2018-11-09 Thread Umann KristĂłf via Phabricator via cfe-commits
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

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits
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

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

2018-11-09 Thread Erik Pilkington via Phabricator via cfe-commits
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() && +

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

2018-11-09 Thread Peter Collingbourne via Phabricator via cfe-commits
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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits
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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits
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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits
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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits
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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Aaron Ballman via Phabricator via 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

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits
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

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits
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

[PATCH] D54372: [analyzer] MisusedMovedObject: NFC: Remove dead code after D18860

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits
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

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits
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

[PATCH] D52296: [Clang] - Add -fdwarf-fission=split,single option.

2018-11-09 Thread David Blaikie via Phabricator via cfe-commits
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

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

2018-11-09 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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() && +!

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits
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

[PATCH] D54373: [clang]: Correct naming conventions and 80 col rule violation in CGDeclCXX.cpp. NFC.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits
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

r346582 - Correct naming conventions and 80 col rule violation in CGDeclCXX.cpp. NFC.

2018-11-09 Thread Kristina Brooks via cfe-commits
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

[PATCH] D54373: [clang]: Correct naming conventions and 80 col rule violation in CGDeclCXX.cpp. NFC.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits
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

<    1   2