[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Thanks for taking a look! ^-^



================
Comment at: lib/StaticAnalyzer/Core/CheckerRegistry.cpp:51
 
-/// Collects the checkers for the supplied \p opt option into \p collected.
-static void collectCheckers(const CheckerRegistry::CheckerInfoList &checkers,
-                            const llvm::StringMap<size_t> &packageSizes,
-                            CheckerOptInfo &opt, CheckerInfoSet &collected) {
-  // Use a binary search to find the possible start of the package.
-  CheckerRegistry::CheckerInfo packageInfo(nullptr, opt.getName(), "");
-  auto end = checkers.cend();
-  auto i = std::lower_bound(checkers.cbegin(), end, packageInfo, checkerNameLT);
-
-  // If we didn't even find a possible package, give up.
-  if (i == end)
-    return;
-
-  // If what we found doesn't actually start the package, give up.
-  if (!isInPackage(*i, opt.getName()))
-    return;
-
-  // There is at least one checker in the package; claim the option.
-  opt.claim();
-
-  // See how large the package is.
-  // If the package doesn't exist, assume the option refers to a single checker.
-  size_t size = 1;
-  llvm::StringMap<size_t>::const_iterator packageSize =
-    packageSizes.find(opt.getName());
-  if (packageSize != packageSizes.end())
-    size = packageSize->getValue();
-
-  // Step through all the checkers in the package.
-  for (auto checkEnd = i+size; i != checkEnd; ++i)
-    if (opt.isEnabled())
-      collected.insert(&*i);
-    else
-      collected.remove(&*i);
+static CheckerInfoSet getEnabledCheckers(
+    const CheckerRegistry::CheckerInfoList &checkers,
----------------
MTC wrote:
> I just curious about the reason why you move the `CheckerInfoSet` from the parameter passing by reference to the return value, keep the number of parameter at a lower value? Or make the code at the caller cite cleaner?
Both, actually. From what I've seen, out-params usually lead to much harder to comprehend code.


https://reviews.llvm.org/D54401