[clang-tidy] DO NOT SUBMIT.. example diff of applying bugprone-argument-comments with AddCommentsToXXXLiterals options

[MyDeveloperDay]

Summary

This review is an example to be used in the review process to show how new options impact a typical clang library (in this case clang-tidy/modernize)

this is the result of applying D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

[MyDeveloperDay]

Update demo diff, with support for Argument Comments for also StringLiteral,CharacterLiterals,UserDefinedLiterals and NullPtrs

clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

 static StringRef getStringFromRange(SourceManager &SourceMgr,
                                     const LangOptions &LangOpts,
                                     SourceRange Range) {
   if (SourceMgr.getFileID(Range.getBegin()) !=
       SourceMgr.getFileID(Range.getEnd())) {
     return StringRef(); // Empty string.
   }
 
-  return Lexer::getSourceText(CharSourceRange(Range, true), SourceMgr,
+  return Lexer::getSourceText(CharSourceRange(Range, /*ITR=*/true), SourceMgr,
                               LangOpts);
 }
 
 /// \brief If the given expression is actually a DeclRefExpr or a MemberExpr,
 /// find and return the underlying ValueDecl; otherwise, return NULL.
 static const ValueDecl *getReferencedVariable(const Expr *E) {
   if (const DeclRefExpr *DRE = getDeclRef(E))
     return dyn_cast<VarDecl>(DRE->getDecl());

 /// next instruction.
 ///
 /// We need to delete a potential newline after the deleted alias, as
 /// clang-format will leave empty lines untouched. For all other formatting we
 /// rely on clang-format to fix it.
 void LoopConvertCheck::getAliasRange(SourceManager &SM, SourceRange &Range) {
   bool Invalid = false;
   const char *TextAfter =
-      SM.getCharacterData(Range.getEnd().getLocWithOffset(1), &Invalid);
+      SM.getCharacterData(Range.getEnd().getLocWithOffset(/*Offset=*/1), &Invalid);
   if (Invalid)
     return;
   unsigned Offset = std::strspn(TextAfter, " \t\r\n");
   Range =
       SourceRange(Range.getBegin(), Range.getEnd().getLocWithOffset(Offset));
 }
 
 /// \brief Computes the changes needed to convert a given for loop, and

clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp

 }
 
 /// \brief Returns true when two Exprs are equivalent.
 bool areSameExpr(ASTContext *Context, const Expr *First, const Expr *Second) {
   if (!First || !Second)
     return false;
 
   llvm::FoldingSetNodeID FirstID, SecondID;
-  First->Profile(FirstID, *Context, true);
-  Second->Profile(SecondID, *Context, true);
+  First->Profile(FirstID, *Context, /*Canonical=*/true);
+  Second->Profile(SecondID, *Context, /*Canonical=*/true);
   return FirstID == SecondID;
 }
 
 /// \brief Returns the DeclRefExpr represented by E, or NULL if there isn't one.
 const DeclRefExpr *getDeclRef(const Expr *E) {
   return dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts());
 }
 
static const Expr *getDereferenceOperand(const Expr *E) {
   return nullptr;
 }
 
 /// \brief Returns true when the Container contains an Expr equivalent to E.
 template <typename ContainerT>
 static bool containsExpr(ASTContext *Context, const ContainerT *Container,
                          const Expr *E) {
   llvm::FoldingSetNodeID ID;
-  E->Profile(ID, *Context, true);
+  E->Profile(ID, *Context, /*Canonical=*/true);
   for (const auto &I : *Container) {
     if (ID == I.second)
       return true;
   }
   return false;
 }
 
 /// \brief Returns true when the index expression is a declaration reference to
void ForLoopIndexUseVisitor::addComponents(const ComponentVector &Components) {
   // FIXME: add sort(on ID)+unique to avoid extra work.
   for (const auto &I : Components)
     addComponent(I);
 }
 
 void ForLoopIndexUseVisitor::addComponent(const Expr *E) {
   llvm::FoldingSetNodeID ID;
   const Expr *Node = E->IgnoreParenImpCasts();
-  Node->Profile(ID, *Context, true);
+  Node->Profile(ID, *Context, /*Canonical=*/true);
   DependentExprs.push_back(std::make_pair(Node, ID));
 }
 
 void ForLoopIndexUseVisitor::addUsage(const Usage &U) {
   SourceLocation Begin = U.Range.getBegin();
   if (Begin.isMacroID())
     Begin = Context->getSourceManager().getSpellingLoc(Begin);
 

clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp

MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name,
                                      StringRef MakeSmartPtrFunctionName)
     : ClangTidyCheck(Name, Context),
       IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
           Options.getLocalOrGlobal("IncludeStyle", "llvm"))),
       MakeSmartPtrFunctionHeader(
           Options.get("MakeSmartPtrFunctionHeader", StdMemoryHeader)),
       MakeSmartPtrFunctionName(
           Options.get("MakeSmartPtrFunction", MakeSmartPtrFunctionName)),
-      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
+      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", /*Default=*/true)) {}
 
 void MakeSmartPtrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IncludeStyle", IncludeStyle);
   Options.store(Opts, "MakeSmartPtrFunctionHeader", MakeSmartPtrFunctionHeader);
   Options.store(Opts, "MakeSmartPtrFunction", MakeSmartPtrFunctionName);
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
Diag << FixItHint::CreateReplacement(
       MakeSmartPtrFunctionName);
 
   // If the smart_ptr is built with brace enclosed direct initialization, use
   // parenthesis instead.
   if (Construct->isListInitialization()) {
     SourceRange BraceRange = Construct->getParenOrBraceRange();
     Diag << FixItHint::CreateReplacement(
         CharSourceRange::getCharRange(
-            BraceRange.getBegin(), BraceRange.getBegin().getLocWithOffset(1)),
+            BraceRange.getBegin(), BraceRange.getBegin().getLocWithOffset(/*Offset=*/1)),
         "(");
     Diag << FixItHint::CreateReplacement(
         CharSourceRange::getCharRange(BraceRange.getEnd(),
-                                      BraceRange.getEnd().getLocWithOffset(1)),
+                                      BraceRange.getEnd().getLocWithOffset(/*Offset=*/1)),
         ")");
   }
 
   insertHeader(Diag, SM.getFileID(ConstructCallStart));
 }
 
 void MakeSmartPtrCheck::checkReset(SourceManager &SM, ASTContext *Ctx,
                                    const CXXMemberCallExpr *Reset,
if (const auto *NewConstruct = New->getConstructExpr()) {
         //   struct S { S(int x); S(); };
         //   smart_ptr<S>(new S{5});
         //   smart_ptr<S>(new S{}); // use default constructor
         // The arguments in the initialization list are going to be forwarded to
         // the constructor, so this has to be replaced with:
         //   std::make_smart_ptr<S>(5);
         //   std::make_smart_ptr<S>();
         InitRange = SourceRange(
-            NewConstruct->getParenOrBraceRange().getBegin().getLocWithOffset(1),
+            NewConstruct->getParenOrBraceRange().getBegin().getLocWithOffset(/*Offset=*/1),
             NewConstruct->getParenOrBraceRange().getEnd().getLocWithOffset(-1));
       }
     } else {
       // Aggregate initialization.
       //   smart_ptr<Pair>(new Pair{first, second});
       // Has to be replaced with:
       //   smart_ptr<Pair>(Pair{first, second});
       //
if (const auto *NewConstruct = New->getConstructExpr()) {
       }
       InitRange = SourceRange(
           New->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
           New->getInitializer()->getSourceRange().getEnd());
     }
     Diag << FixItHint::CreateRemoval(
         CharSourceRange::getCharRange(NewStart, InitRange.getBegin()));
     Diag << FixItHint::CreateRemoval(
-        SourceRange(InitRange.getEnd().getLocWithOffset(1), NewEnd));
+        SourceRange(InitRange.getEnd().getLocWithOffset(/*Offset=*/1), NewEnd));
     break;
   }
   }
   return true;
 }
 
 void MakeSmartPtrCheck::insertHeader(DiagnosticBuilder &Diag, FileID FD) {
   if (MakeSmartPtrFunctionHeader.empty()) {

clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp

for (const FunctionDecl *Redecl : Ctor->redecls())
     Results.push_back(Redecl->getParamDecl(ParamIdx));
   return Results;
 }
 
 PassByValueCheck::PassByValueCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
           Options.getLocalOrGlobal("IncludeStyle", "llvm"))),
-      ValuesOnly(Options.get("ValuesOnly", 0) != 0) {}
+      ValuesOnly(Options.get("ValuesOnly", /*Default=*/0) != 0) {}
 
 void PassByValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IncludeStyle",
                 utils::IncludeSorter::toString(IncludeStyle));
   Options.store(Opts, "ValuesOnly", ValuesOnly);
 }
 
 void PassByValueCheck::registerMatchers(MatchFinder *Finder) {
std::string ValueStr = Lexer::getSourceText(CharSourceRange::getTokenRange(
                                .str();
     ValueStr += ' ';
     Diag << FixItHint::CreateReplacement(TypeRange, ValueStr);
   }
 
   // Use std::move in the initialization list.
   Diag << FixItHint::CreateInsertion(Initializer->getRParenLoc(), ")")
        << FixItHint::CreateInsertion(
-              Initializer->getLParenLoc().getLocWithOffset(1), "std::move(");
+              Initializer->getLParenLoc().getLocWithOffset(/*Offset=*/1), "std::move(");
 
   if (auto IncludeFixit = Inserter->CreateIncludeInsertion(
           Result.SourceManager->getFileID(Initializer->getSourceLocation()),
           "utility",
           /*IsAngled=*/true)) {
     Diag << *IncludeFixit;
   }
 }
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang

clang-tools-extra/clang-tidy/modernize/RawStringLiteralCheck.cpp

 }
 
 } // namespace
 
 RawStringLiteralCheck::RawStringLiteralCheck(StringRef Name,
                                              ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       DelimiterStem(Options.get("DelimiterStem", "lit")),
-      ReplaceShorterLiterals(Options.get("ReplaceShorterLiterals", false)) {
+      ReplaceShorterLiterals(Options.get("ReplaceShorterLiterals", /*Default=*/false)) {
   // Non-printing characters are disallowed:
   // \007 = \a bell
   // \010 = \b backspace
   // \011 = \t horizontal tab
   // \012 = \n new line
   // \013 = \v vertical tab
   // \014 = \f form feed
   // \015 = \r carriage return

clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp

if (State == SawVoid && ProtoToken.is(tok::TokenKind::r_paren)) {
     removeVoidToken(VoidToken, Diagnostic);
   }
 }
 
 void RedundantVoidArgCheck::removeVoidToken(Token VoidToken,
                                             StringRef Diagnostic) {
   SourceLocation VoidLoc(VoidToken.getLocation());
   auto VoidRange =
-      CharSourceRange::getTokenRange(VoidLoc, VoidLoc.getLocWithOffset(3));
+      CharSourceRange::getTokenRange(VoidLoc, VoidLoc.getLocWithOffset(/*Offset=*/3));
   diag(VoidLoc, Diagnostic) << FixItHint::CreateRemoval(VoidRange);
 }
 
 void RedundantVoidArgCheck::processTypedefNameDecl(
     const MatchFinder::MatchResult &Result,
     const TypedefNameDecl *TypedefName) {
   if (protoTypeHasNoParms(TypedefName->getUnderlyingType())) {
     removeVoidArgumentTokens(Result, TypedefName->getSourceRange(),

clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp

return declStmt(
       anyOf(makeIteratorDeclMatcher(), makeDeclWithNewMatcher(),
             makeDeclWithCastMatcher(), makeDeclWithTemplateCastMatcher()));
 }
 
 } // namespace
 
 UseAutoCheck::UseAutoCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      MinTypeNameLength(Options.get("MinTypeNameLength", 5)),
-      RemoveStars(Options.get("RemoveStars", 0)) {}
+      MinTypeNameLength(Options.get("MinTypeNameLength", /*Default=*/5)),
+      RemoveStars(Options.get("RemoveStars", /*Default=*/0)) {}
 
 void UseAutoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "MinTypeNameLength", MinTypeNameLength);
   Options.store(Opts, "RemoveStars", RemoveStars ? 1 : 0);
 }
 
 void UseAutoCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
for (const auto *Dec : D->decls()) {
     }
 
     if (const auto *NestedConstruct = dyn_cast<CXXConstructExpr>(E)) {
       // If we ran into an implicit conversion contructor, can't convert.
       //
       // FIXME: The following only checks if the constructor can be used
       // implicitly, not if it actually was. Cases where the converting
       // constructor was used explicitly won't get converted.
-      if (NestedConstruct->getConstructor()->isConvertingConstructor(false))
+      if (NestedConstruct->getConstructor()->isConvertingConstructor(/*AllowExplicit=*/false))
         return;
     }
     if (!Context->hasSameType(V->getType(), E->getType()))
       return;
   }
 
   // Get the type location using the first declaration.
   const auto *V = cast<VarDecl>(*D->decl_begin());

clang-tools-extra/clang-tidy/modernize/UseBoolLiteralsCheck.cpp

 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
 UseBoolLiteralsCheck::UseBoolLiteralsCheck(StringRef Name,
                                            ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
+      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", /*Default=*/true)) {}
 
 void UseBoolLiteralsCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
 
   Finder->addMatcher(
       implicitCastExpr(
           has(ignoringParenImpCasts(integerLiteral().bind("literal"))),

clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

static bool sameValue(const Expr *E1, const Expr *E2) {
   default:
     return false;
   }
 }
 
 UseDefaultMemberInitCheck::UseDefaultMemberInitCheck(StringRef Name,
                                                      ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      UseAssignment(Options.get("UseAssignment", 0) != 0),
-      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true) != 0) {}
+      UseAssignment(Options.get("UseAssignment", /*Default=*/0) != 0),
+      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", /*Default=*/true) != 0) {}
 
 void UseDefaultMemberInitCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "UseAssignment", UseAssignment);
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UseDefaultMemberInitCheck::registerMatchers(MatchFinder *Finder) {

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

 const auto DefaultSmartPointers =
     "::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
 const auto DefaultTupleTypes = "::std::pair; ::std::tuple";
 const auto DefaultTupleMakeFunctions = "::std::make_pair; ::std::make_tuple";
 } // namespace
 
 UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IgnoreImplicitConstructors(Options.get("IgnoreImplicitConstructors", 0)),
+      IgnoreImplicitConstructors(Options.get("IgnoreImplicitConstructors", /*Default=*/0)),
       ContainersWithPushBack(utils::options::parseStringList(Options.get(
           "ContainersWithPushBack", DefaultContainersWithPushBack))),
       SmartPointers(utils::options::parseStringList(
           Options.get("SmartPointers", DefaultSmartPointers))),
       TupleTypes(utils::options::parseStringList(
           Options.get("TupleTypes", DefaultTupleTypes))),
       TupleMakeFunctions(utils::options::parseStringList(
           Options.get("TupleMakeFunctions", DefaultTupleMakeFunctions))) {}

clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp

static bool isCopyAssignmentAndCanBeDefaulted(ASTContext *Context,
   // Ensure that we don't do anything else.
   return Compound->size() == BasesToInit.size() + FieldsToInit.size() + 1;
 }
 
 /// \brief Returns false if the body has any non-whitespace character.
 static bool bodyEmpty(const ASTContext *Context, const CompoundStmt *Body) {
   bool Invalid = false;
   StringRef Text = Lexer::getSourceText(
-      CharSourceRange::getCharRange(Body->getLBracLoc().getLocWithOffset(1),
+      CharSourceRange::getCharRange(Body->getLBracLoc().getLocWithOffset(/*Offset=*/1),
                                     Body->getRBracLoc()),
       Context->getSourceManager(), Context->getLangOpts(), &Invalid);
   return !Invalid && std::strspn(Text.data(), " \t\r\n") == Text.size();
 }
 
 UseEqualsDefaultCheck::UseEqualsDefaultCheck(StringRef Name,
                                              ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true) != 0) {}
+      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", /*Default=*/true) != 0) {}
 
 void UseEqualsDefaultCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;

clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp

 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
 UseNoexceptCheck::UseNoexceptCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       NoexceptMacro(Options.get("ReplacementString", "")),
-      UseNoexceptFalse(Options.get("UseNoexceptFalse", true)) {}
+      UseNoexceptFalse(Options.get("UseNoexceptFalse", /*Default=*/true)) {}
 
 void UseNoexceptCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "ReplacementString", NoexceptMacro);
   Options.store(Opts, "UseNoexceptFalse", UseNoexceptFalse);
 }
 
 void UseNoexceptCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)

clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp

bool isReplaceableRange(SourceLocation StartLoc, SourceLocation EndLoc,
   return SM.isWrittenInSameFile(StartLoc, EndLoc);
 }
 
 /// \brief Replaces the provided range with the text "nullptr", but only if
 /// the start and end location are both in main file.
 /// Returns true if and only if a replacement was made.
 void replaceWithNullptr(ClangTidyCheck &Check, SourceManager &SM,
                         SourceLocation StartLoc, SourceLocation EndLoc) {
-  CharSourceRange Range(SourceRange(StartLoc, EndLoc), true);
+  CharSourceRange Range(SourceRange(StartLoc, EndLoc), /*ITR=*/true);
   // Add a space if nullptr follows an alphanumeric character. This happens
   // whenever there is an c-style explicit cast to nullptr not surrounded by
   // parentheses and right beside a return statement.
   SourceLocation PreviousLocation = StartLoc.getLocWithOffset(-1);
   bool NeedsSpace = isAlphanumeric(*SM.getCharacterData(PreviousLocation));
   Check.diag(Range.getBegin(), "use nullptr") << FixItHint::CreateReplacement(
       Range, NeedsSpace ? " nullptr" : "nullptr");
 }

clang-tools-extra/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp

 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
 UseTransparentFunctorsCheck::UseTransparentFunctorsCheck(
     StringRef Name, ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context), SafeMode(Options.get("SafeMode", 0)) {}
+    : ClangTidyCheck(Name, Context), SafeMode(Options.get("SafeMode", /*Default=*/0)) {}
 
 void UseTransparentFunctorsCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "SafeMode", SafeMode ? 1 : 0);
 }
 
 void UseTransparentFunctorsCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus14)

clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp

 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
+      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", /*Default=*/true)) {}
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
     return;
   Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
                      this);
 }