[PATCH] D67056: Add a bugprone-argument-comment option: IgnoreSingleArgument.

Wed Sep 4 10:23:57 PDT 2019

xyb marked 2 inline comments as done.
xyb added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp:28
+      IgnoreSingleArgument(
+          Options.getLocalOrGlobal("IgnoreSingleArgument", 0) != 0),
       CommentBoolLiterals(Options.getLocalOrGlobal("CommentBoolLiterals", 0) !=
----------------
alexfh wrote:
> xyb wrote:
> > alexfh wrote:
> > > Why is it a global option? Are there other checks that would need the same option? The one below also needs to be check-local.
> > Sorry, I'm afraid I didn't get your point. Could you please explain more? This setting just follows the same pattern used for other settings. All other settings use "Options.getLocalOrGlobal(...)", I'm not sure why this setting need be different. Or do you mean we should change other settings ("StrictMode", "CommentBoolLiterals", "CommentIntegerLiterals", ...) to local options also?
> Options are stored as a string -> string map. The key in this map is either the option name prepended with the check name (for check-local options) or just the option name (this way an option can be read by all checks). There are two ways to read options: OptionsView::get reads just the local name, OptionsView::getLocalOrGlobal tries the check-local name and then falls back to reading the option using its global name.
> 
> In this particular check only the StrictMode option makes sense to be global (a few other checks define an option with the same name and set of values, and it may make sense to configure a global default value). Other options are specific to this check and should be local.
Thanks for your explanation! Updated. 

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67056/new/

https://reviews.llvm.org/D67056