[llvm-branch-commits] [clang] [SSAF][UnsafeBufferUsage] Implement AST visitor that respects the contribution model (PR #188652)

[Balázs Benics via llvm-branch-commits]

================
@@ -0,0 +1,148 @@
+//===-- SSAFAnalysesCommon.h ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  Common code in SSAF analyses implementations
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_ANALYSES_SSAFANALYSESCOMMON_H
+#define LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_ANALYSES_SSAFANALYSESCOMMON_H
+
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/JSON.h"
+
+using namespace clang;
+
+template <typename NodeTy, typename... Ts>
+static inline llvm::Error makeErrAtNode(ASTContext &Ctx, const NodeTy &N,
+                                        StringRef Fmt, const Ts &...Args) {
+  std::string LocStr = N.getBeginLoc().printToString(Ctx.getSourceManager());
+  llvm::SmallVector<char> FmtData;
+
+  (Fmt + " at %s").toStringRef(FmtData);
+  return llvm::createStringError(FmtData.data(), Args..., LocStr.c_str());
----------------
steakhal wrote:

Uhh, `toStringRef` wasn't an API I was used to. I had to look what it does and makes me worried.
```c++
  StringRef toStringRef(SmallVectorImpl<char> &Out) const {
    if (isSingleStringRef())
      return getSingleStringRef();
    toVector(Out);
    return StringRef(Out.data(), Out.size());
  }
```

This means that the `Out` buffer is only used if the Twine isn't a single StringRef (aka. the common case; and the case here because it's a concatenation). This means that `FmtData` will remain default constructed (aka. empty), and we pass it as a format string and feed it with at least one argument. This should have fired a runtime assert. (or llvm::format doesn't check for unused format args?)

BTW there is a `createStringError` overload taking Twines, exactly for this; so the solution seems to be just:


```suggestion
  return llvm::createStringError(Fmt + " at %s", Args..., LocStr.c_str());
```

https://github.com/llvm/llvm-project/pull/188652