[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

This revision was automatically updated to reflect the committed changes. Closed by commit rL358428: [CommandLineParser] Add DefaultOption flag (authored by dhinton, committed by ). Changed prior to commit: https://reviews.llvm.org/D59746?vs=195218=195220#toc Repository: rL LLVM CHANGES

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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/

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

This revision was automatically updated to reflect the committed changes. Closed by commit rL358337: [CommandLineParser] Add DefaultOption flag (authored by dhinton, committed by ). Herald added a subscriber: kristina. Changed prior to commit:

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg 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

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:

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

klimek added inline comments. Comment at: llvm/lib/Support/CommandLine.cpp:101 + SmallVector DefaultOptions; + A comment explaining what this contains and how it'll be used would help. Comment at: llvm/lib/Support/CommandLine.cpp:152 +

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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

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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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 :

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

jhenderson added a comment. 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 that test their own short alias interpretation somewhere though. Repository: rG

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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