[PATCH] D55707: Update to SARIF 11-28

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 14 12:46:12 PST 2018


NoQ added a comment.

Sure, np.



================
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:129
+  unsigned Index = 0;
+  for (const json::Value &File : Files) {
+    if (const json::Object *Obj = File.getAsObject()) {
----------------
aaron.ballman wrote:
> NoQ wrote:
> > This sounds like `find_if` to me.
> The problem is: we need the `Index`. It felt a bit weird to have `llvm::find_if()` (a const operation) also mutating a locally-captured variable. WDYT?
The way i see it:

```
auto I = std::find_if(Files.begin(), Files.end(), [&](const json::Value &File {
  if (const json::Object *Obj = File.getAsObject())
    if (const json::Object *FileLoc = Obj->getObject("fileLocation")) {
      Optional<StringRef> URI = FileLoc->getString("uri");
      return URI && URI->equals(FileURI);
    }
  return false;
});

if (I == Files.end())
  Files.push_back(createFile(FE));

return std::distance(Files.begin(), I);
```

Or pre-compute the index before `push_back` if the container invalidates iterators upon `push_back`.



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

https://reviews.llvm.org/D55707





More information about the cfe-commits mailing list