[PATCH] D55707: Update to SARIF 11-28

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:129
+  unsigned Index = 0;
+  for (const json::Value &File : Files) {
+    if (const json::Object *Obj = File.getAsObject()) {
----------------
NoQ wrote:
> 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`.
> 
I dig it! Implemented in r349197.


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

https://reviews.llvm.org/D55707