[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

2019-04-02 Thread Don Hinton via Phabricator via cfe-commits
hintonda created this revision. hintonda added a reviewer: alexfh. Herald added a subscriber: xazax.hun. Herald added a project: clang. Fixed ambiguous namespace bug in llvm checkers that add the llvm namespace to clang::tidy, by adding using declaration for SmallSet. Repository: rG LLVM Githu

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

2019-04-02 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 193376. hintonda added a comment. - Renamed and updated docs. - Fix formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files: clang-tools-extra/clang-tidy/l

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-02 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 193379. hintonda added a comment. - Remove spaces in docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files: clang-tools-extra/clang-tidy/llvm/CMakeLists.txt

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

2019-04-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D60151#1453099 , @alexfh wrote: > The change looks fine, but I don't understand the description of this > revision. Could you clarify which checkers you're talking about and which bug > you observe? Sorry for not being clea

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

2019-04-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21 -typedef llvm::SmallSet HeaderFileExtensionsSet; +using HeaderFileExtensionsSet = SmallSet; aaron.ballman

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

2019-04-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21 -typedef llvm::SmallSet HeaderFileExtensionsSet; +using HeaderFileExtensionsSet = SmallSet; aaron.ballman

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

2019-04-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21 -typedef llvm::SmallSet HeaderFileExtensionsSet; +using HeaderFileExtensionsSet = SmallSet; aaron.ballman

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

2019-04-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 193604. hintonda added a comment. Herald added subscribers: arphaman, mgorny. - Rename llvm directory to llvm_project. - Change llvm- to llvm-project-. - Rename files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145 + +diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null") +<< FixItHint::CreateReplacement(SourceRange(Match

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145 + +diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null") +<< FixItHint::CreateReplacement(SourceRange(Match

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18 + +AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); } +} // namespace ast_matchers aaro

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

2019-04-04 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D60151#1454741 , @alexfh wrote: > In D60151#1454105 , @hintonda wrote: > > > - Rename llvm directory to llvm_project. > > - Change llvm- to llvm-project-. > > - Rename files. > > > Aweso

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-04 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18 + +AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); } +} // namespace ast_matchers aaro

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-05 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59746#1456115 , @alexfh wrote: > Can you give a specific example of how this problem manifests? Any tool using `tooling::CommonOptionsParser` with the `llvm::cl::OneOrMore` flag will have this problem, i.e., the `-h` flag w

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-05 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 193968. hintonda added a comment. - Use new isa_and_nonnull, and cleanup code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files: clang-tools-extra/clang-tidy/ll

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-06 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194017. hintonda added a comment. Explicitly check for implicitCastExpr and add additional negative matching test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 File

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-08 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59746#1458086 , @klimek wrote: > In D59746#1440756 , @hintonda wrote: > > > A better alternative would have been to add a cl::aliasopt for '-h' in > > llvm's CommandLineParser when '-h

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-08 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59746#1458432 , @klimek wrote: > In D59746#1458424 , @hintonda wrote: > > > In D59746#1458086 , @klimek wrote: > > > > > In D59746#1440756

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-08 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59746#1458461 , @hintonda wrote: > In D59746#1458432 , @klimek wrote: > > > If we make it an alias by default, can somebody overwrite that? > > > Unfortunately, that produces a runtime

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-09 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59746#1459672 , @klimek wrote: > In D59746#1458539 , @hintonda wrote: > > > In D59746#1458461 , @hintonda > > wrote: > > > > > In D59746#145843

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-09 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59746#1459826 , @hintonda wrote: > In D59746#1459672 , @klimek wrote: > > > In D59746#1458539 , @hintonda > > wrote: > > > > > In D59746#145846

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-09 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59746#1459906 , @hintonda wrote: > In D59746#1459826 , @hintonda wrote: > > > In D59746#1459672 , @klimek wrote: > > > > > If we can extend cl::

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-09 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. Have a working prototype for aliases, but underneath, an option is an option, so should be able to support both without any trouble. Will upload as soon as I clean it up and add tests, etc... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-10 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194539. hintonda added a comment. Herald added subscribers: llvm-commits, hiraditya. Herald added a project: LLVM. - Add DefaultOption logic. Still needs tests, but wanted to get early feedback on basic approach. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D59746: [LibTooling] Fix '-h' option

2019-04-10 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: llvm/lib/Support/CommandLine.cpp:1187 +// FIXME: This is needed, but not sure why. +updateArgStr(Opt, NewArg, ChosenSubCommand); +Opt->setArgStr(NewArg); ---

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

2019-04-10 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. ping, and should we add new warning messages as suggested by @Rakete ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59754/new/ https://reviews.llvm.org/D59754 ___ cfe-com

[PATCH] D17407: [Sema] PR25755 Handle out of order designated initializers

2019-04-10 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. ping... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D17407/new/ https://reviews.llvm.org/D17407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-10 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194633. hintonda edited the summary of this revision. hintonda added a comment. - Fix bug in updateArgStr() where it didn't handle isInAllSubCommands() correctly, and refactored code based on how SubCommands work. Repository: rG LLVM Github Monorepo CHA

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194669. hintonda added a comment. - Delay actually adding default options until ParseCommandLineOptions which simplifies handling and makes it easy to only add them to the correct SubCommand. Add simple tests. Repository: rG LLVM Github Monorepo CHA

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked 2 inline comments as done. hintonda added inline comments. Comment at: llvm/lib/Support/CommandLine.cpp:282 else { - for (auto SC : O->Subs) -updateArgStr(O, NewName, SC); + if (O->isInAllSubCommands()) { +for (auto SC : RegisteredSu

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59746#1462352 , @jhenderson wrote: > I've not looked at the implementation in depth, but cl::DefaultOption seems > like a nice way of handling this. Please make sure that there is testing in > llvm-readobj and llvm-objdump t

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194708. hintonda added a comment. - Add unittest. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D59746 Files: clang/lib/Tooling/CommonOptionsParser.cpp llvm/docs/Comman

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D60151#1462850 , @MyDeveloperDay wrote: > are we supporting "-llvm-*" in existing .clang-tidy files? > > If people selectively turn checkers off, won't all of a sudden everyone start > getting llvm-project checks and fixes tu

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194729. hintonda added a comment. Enhance Option::reset to reset the default value and remove the option if it's a cl::DefaultOption. Also, make sure removeOption only removes an option if both the name and the Option matches, not just the name -- different

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. @klimek, this patch is now ready for review. Thanks... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D59746 ___ cfe-commits mailing list c

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D60151#1463484 , @MyDeveloperDay wrote: > >> I suppose we could keep the names and directory structure and just change > >> the namespace. That would just be a special case in the scripts. Haven't > >> looked into it yet,

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h:37-39 +/// if (var && isa(var)) {} +/// // is replaced by: +/// if (dyn_cast_or_null(var.foo())) {} ---

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-11 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194782. hintonda added a comment. - Update isa_and_nonnull diagnostic message, and fix typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files: clang-tools-extra

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-12 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194899. hintonda added a comment. - Fix comments and add isDefaultOption per review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D59746 Files: clang/lib/Toolin

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

2019-04-12 Thread Don Hinton via Phabricator via cfe-commits
hintonda abandoned this revision. hintonda added a comment. Replaced by D60629 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60151/new/ https://reviews.llvm.org/D60151 __

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_checker'

2019-04-12 Thread Don Hinton via Phabricator via cfe-commits
hintonda created this revision. hintonda added reviewers: alexfh, aaron.ballman. Herald added a subscriber: xazax.hun. Herald added a project: clang. Change the namespace for llvm checkers from 'llvm' to 'llvm_checker', and modify add_new_check.py and rename_check.py to support the new namespace.

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_checker'

2019-04-12 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194941. hintonda added a comment. - Add missing 'clang'. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60629/new/ https://reviews.llvm.org/D60629 Files: clang-tools-extra/clang-tidy/add_new_check.py clang

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_checker'

2019-04-12 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 194940. hintonda added a comment. - Remove leftover comments, etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60629/new/ https://reviews.llvm.org/D60629 Files: clang-tools-extra/clang-tidy/add_new_check.

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_checker'

2019-04-12 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D60629#1464945 , @Eugene.Zelenko wrote: > Please use check for consistency with rest of Clang-tidy. Thanks for taking a look — I’ll Fix that on the next upload. So are you okay with llvm_check? I sorta like it, but happy

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-13 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59746#1465445 , @klimek wrote: > lg Thanks again for the help and the suggestion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D59746

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-15 Thread Don Hinton via Phabricator via cfe-commits
hintonda reopened this revision. hintonda added a comment. This revision is now accepted and ready to land. Reopen to track fix after buildbot failure and revert -- r358414. http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190415/644037.html Repository: rL LLVM CHANGES SINCE LAST A

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-15 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 195217. hintonda added a comment. - Original patch, r358337, that was reverted. - Make sure to clear DefaultOptions on reset. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-15 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 195218. hintonda added a comment. - Add new test back. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D59746 Files: clang/lib/Tooling/CommonOptionsParser.cpp llvm/docs/C

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-15 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added a comment. In D59802#1467363 , @aaron.ballman wrote: > LGTM aside from a nit and the ultimate name for the `isa_and_nonnull<>` API. Thanks. I'll ping the list again at the end of the week, then

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-15 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:382 + if module == 'llvm' or module == 'clang': +namespace = module + '_checker' + else: aaron.ballman wrote: > I thought we

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-15 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 195270. hintonda added a comment. - Alphabetize registration calls. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files: clang-tools-extra/clang-tidy/llvm/CMakeLis

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-15 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 195278. hintonda added a comment. - Change `llvm_checker` to `llvm_check`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60629/new/ https://reviews.llvm.org/D60629 Files: clang-tools-extra/clang-tidy/add_ne

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-16 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28 CheckFactories.registerCheck("llvm-include-order"); +CheckFactories.registerCheck( +"llvm-prefer-isa-or-dyn-cast-in-c

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-17 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28 CheckFactories.registerCheck("llvm-include-order"); +CheckFactories.registerCheck( +"llvm-prefer-isa-or-dyn-cast-in-c

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-17 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 195581. hintonda added a comment. - Remove clang module check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60629/new/ https://reviews.llvm.org/D60629 Files: clang-tools-extra/clang-tidy/add_new_check.py

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-17 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked 2 inline comments as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:381 + # Don't allow 'clang' or 'llvm' namespaces + if module == 'llvm' or module == 'clang': +namespace = module + '_check'

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

2019-04-17 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 195610. hintonda added a comment. - Removed auto and added specific warnings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59754/new/ https://reviews.llvm.org/D59754 Files: clang/include/clang/Basic/Diagno

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

2019-04-17 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 195611. hintonda added a comment. - Fix typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59754/new/ https://reviews.llvm.org/D59754 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sem

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

2019-04-17 Thread Don Hinton via Phabricator via cfe-commits
hintonda added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + IgnoreListInit(Options.get("IgnoreListInit", false)) {} You’re setting it false here. CHANGES

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

2019-04-17 Thread Don Hinton via Phabricator via cfe-commits
hintonda added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + IgnoreListInit(Options.get("IgnoreListInit", false)) {} axzhang wrote: > hintonda wrote: > > Yo

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

2019-04-17 Thread Don Hinton via Phabricator via cfe-commits
hintonda added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + IgnoreListInit(Options.get("IgnoreListInit", false)) {} hintonda wrote: > axzhang wrote: > > hi

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

2019-04-18 Thread Don Hinton via Phabricator via cfe-commits
hintonda added inline comments. Comment at: clang-tidy/abseil/AbseilTidyModule.cpp:77 +Opts["abseil-make-unique.MakeSmartPtrFunction"] = "absl::make_unique"; +Opts["abseil-make-unique.IgnoreListInit"] = true; +return Options; This is defined as `typed

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-21 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/rename_check.py:218 header_guard_variants = [ + (args.old_check_name.replace('-', '_') + '_Check').upper(), + (old_module + '_' + check_name_camel).upp

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-21 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 196011. hintonda added a comment. - Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60629/new/ https://reviews.llvm.org/D60629 Files: clang-tools-extra/clang-tidy/add_new_check.py clang-to

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

2019-04-21 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:2069-2072 +if (!VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a) { + SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

2019-04-22 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:2069-2072 +if (!VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a) { + SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-22 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. @aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, and it seems to work fine. However, it did miss this one: - if (V && isa(V) && (EntInst = cast(V)) && +if (isa_and_nonnull(V) && (EntInst = cast(V)) && It got the first, but not the

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-23 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D59802#1475596 , @aaron.ballman wrote: > In D59802#1474300 , @hintonda wrote: > > > @aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, > > and it seems to work

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 196521. hintonda added a comment. - Remove unnecessary comparison. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60629/new/ https://reviews.llvm.org/D60629 Files: clang-tools-extra/clang-tidy/add_new_check.

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 196531. hintonda added a comment. - Change check back to previous version which is much clearer. - Apply namespace change to new check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60629/new/ https://reviews.

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/rename_check.py:267 - if old_module != new_module: + if old_module != new_module or new_module == 'llvm': +if new_module == 'llvm': alexfh

[PATCH] D60910: [WIP] Dumping the AST to JSON

2019-04-25 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. Looks good, but just a couple of questions. Comment at: include/clang/AST/JSONNodeDumper.h:73 + Indentation.clear(); + OS << "\n" << Indentation << "}\n"; + TopLevel = true; Just curious, since you clear Indentation on t

[PATCH] D60910: [WIP] Dumping the AST to JSON

2019-04-25 Thread Don Hinton via Phabricator via cfe-commits
hintonda added inline comments. Comment at: include/clang/Frontend/FrontendOptions.h:311 + /// Specifies the output format of the AST. + enum ASTOutputFormat { +AOF_Default, aaron.ballman wrote: > hintonda wrote: > > Why can't you use the enum defined in `c

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

2019-04-30 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 197267. hintonda added a comment. Herald added subscribers: cfe-commits, thopre. Herald added a project: clang. - Fix dashes in error messages and in tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61269/n

[PATCH] D61270: [CommandLine] Enable Grouping for short options by default. Part 4 of 5

2019-04-30 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 197271. hintonda added a comment. Herald added subscribers: cfe-commits, thopre. Herald added a project: clang. - Fix test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61270/new/ https://reviews.llvm.org/D61

[PATCH] D61270: [CommandLine] Enable Grouping for short options by default. Part 4 of 5

2019-04-30 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 197272. hintonda added a comment. Minimize diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61270/new/ https://reviews.llvm.org/D61270 Files: llvm/include/llvm/Support/CommandLine.h llvm/lib/Support/Co

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

2019-04-30 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: llvm/lib/Support/CommandLine.cpp:95 + +static size_t argPrefixesSize(size_t len) { + if (len == 1) thopre wrote: > Any reason why not take a StringRef of the option to compute

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

2019-04-30 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 197466. hintonda added a comment. - Refactor and fix additional dashes in error messages and tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61269/new/ https://reviews.llvm.org/D61269 Files: clang/test

[PATCH] D61270: [CommandLine] Enable Grouping for short options by default. Part 4 of 5

2019-04-30 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D61270#1483661 , @MaskRay wrote: > Thank you for the patch series! > > > This is consistent with GNU getopt behavior. > > This is not GNU specific :) It is POSIX Utility Syntax Guidelines 5 > http://pubs.opengroup.org/online

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-30 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. Ping. Is this the direction you'd like to go? Or would you prefer something else? thanks... don Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60629/new/ https://reviews.llvm.org/D60629 __

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

2019-05-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D61269#1490176 , @thakis wrote: > I happened to see this go by. Is there an explanation of the overall goal > somewhere? In general, requiring -- for long flags sounds like a great change > to me, but there are a few exceptio

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-05-07 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. ping... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60629/new/ https://reviews.llvm.org/D60629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-05-08 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D60629#1494908 , @aaron.ballman wrote: > This LGTM, but you may want to wait a day or so to see if @alexfh has any > remaining concerns. Will do, thanks Aaron... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

2019-05-08 Thread Don Hinton via Phabricator via cfe-commits
hintonda created this revision. hintonda added a reviewer: zturner. Herald added subscribers: llvm-commits, delcypher. Herald added projects: clang, LLVM. This test fails to link shared libraries because tries to run a copied version of clang-check to see if the mock version of libcxx in the same

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

2019-05-08 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. @zturner, I only disabled this on Darwin because that's all I can test on right now. Please let me know if there's a better way to do this or if it should fire on different platforms. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

2017-09-04 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. ping... https://reviews.llvm.org/D36347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git

2017-09-06 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. Now that 36971 has been committed, I believe you can just remove the `find_first_existing_file` and `find_first_existing_vc_file` macro definitions from this file. https://reviews.llvm.org/D35533 ___ cfe-commits mailing l

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git

2017-09-06 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In https://reviews.llvm.org/D35533#862798, @minseong.kim wrote: > Hi~ @hintonda, > > Using using find_first_existing_file in ADDLLVM.cmake solves the cases with > repo in conjunction with https://reviews.llvm.org/D35532. However, I am not > sure it can handle @modocach

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git

2017-09-06 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. I just looked into both approaches and believe the one taken in AddLLVM.cmake is correct and handles the submodule case correctly. It uses `git rev-parse --git-dir` to find the location of the actual .git directory. Please see https://reviews.llvm.org/D31985 for detai

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git

2017-09-06 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In https://reviews.llvm.org/D35533#862852, @minseong.kim wrote: > @hintonda, Absolutely. Incorporating @modocache's module changes into the > version in AddLLVM.cmake would solve the current version display issue for > repo and do not affect the process of other versio

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git

2017-09-07 Thread Don Hinton via Phabricator via cfe-commits
hintonda accepted this revision. hintonda added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

2019-05-31 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. ping... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61697/new/ https://reviews.llvm.org/D61697 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D62445: [test] Fix plugin tests

2019-06-01 Thread Don Hinton via Phabricator via cfe-commits
hintonda reopened this revision. hintonda added a comment. This revision is now accepted and ready to land. This patch broke some of the bots, so reopening to track the fix. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62445/new/ https://reviews.llvm.org/D62445

[PATCH] D62445: [test] Fix plugin tests

2019-06-01 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 202573. hintonda added a comment. - Fix dependencies so required plugins get built before tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62445/new/ https://reviews.llvm.org/D62445 Files: clang/lib/Ana

[PATCH] D62445: [test] Fix plugin tests

2019-06-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D62445#1527368 , @davezarzycki wrote: > This broke non-PIC builds. Fixed: r362399 Sorry for the breakage, and thanks for the fix! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62445/new/ htt

[PATCH] D62445: [test] Fix plugin tests

2019-06-04 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: cfe/trunk/lib/Analysis/plugins/CMakeLists.txt:1 +if(LLVM_ENABLE_PLUGINS) + add_subdirectory(SampleAnalyzer) nathanchance wrote: > I think this should have a dependency on `CLA

[PATCH] D62445: [test] Fix plugin tests

2019-06-04 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done. hintonda added inline comments. Comment at: cfe/trunk/lib/Analysis/plugins/CMakeLists.txt:1 +if(LLVM_ENABLE_PLUGINS) + add_subdirectory(SampleAnalyzer) hintonda wrote: > nathanchance wrote: > > I think this should have

[PATCH] D62873: Avoid building analyzer plugins if CLANG_ENABLE_STATIC_ANALYZER is OFF

2019-06-04 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In D62873#1529800 , @nathanchance wrote: > Thanks for the quick fix, looks good to me! Ah, sorry, just saw this. I just committed r362555 that addresses this. Repository: rC Clang CHANGES SINCE LAST ACTION https://revie

[PATCH] D62873: Avoid building analyzer plugins if CLANG_ENABLE_STATIC_ANALYZER is OFF

2019-06-04 Thread Don Hinton via Phabricator via cfe-commits
hintonda accepted this revision. hintonda added a comment. This revision is now accepted and ready to land. LGTM... Comment at: test/CMakeLists.txt:122 -if (CLANG_ENABLE_STATIC_ANALYZER) - if (LLVM_ENABLE_PLUGINS) -list(APPEND CLANG_TEST_DEPS - SampleAnalyzerPlugin

[PATCH] D62873: Avoid building analyzer plugins if CLANG_ENABLE_STATIC_ANALYZER is OFF

2019-06-04 Thread Don Hinton via Phabricator via cfe-commits
hintonda added inline comments. Comment at: lib/Analysis/plugins/CMakeLists.txt:1 -if(LLVM_ENABLE_PLUGINS) +if(LLVM_ENABLE_PLUGINS AND CLANG_ENABLE_STATIC_ANALYZER) add_subdirectory(SampleAnalyzer) Szelethus wrote: > Is this file a thing? `lib/Analysis/plugins

<    1   2   3   4   >