[PATCH] D121533: [clang][deps] Fix traversal of precompiled dependencies

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

I'm not a big fan of moving to the recursive call. Creating a separate `StringMap` for each iteration seems awfully wasteful.

I'd prefer fixing the iterative algorithm so that it's correct and easy to verify. This can be done with a stack that points at the stable pointers into the StringMap:

  StringMap<std::string> Imports;
  SmallVector<StringMapEntry<std::string> *> Worklist;
  PrebuiltModuleListener Listener(Imports, Worklist, InputFiles, VisitInputFiles);
  
  while (!Worklist.empty()) {
    StringMapEntry<std::string> *Import = Worklist.pop_back_val();
    // Deal with "Import".
  }

and in `PrebuiltModuleListener` only push to the worklist for newly-discovered imports:

  void visitImport(StringRef ModuleName, StringRef Filename) override {
    auto I = PrebuiltModuleFiles.insert({ModuleName, Filename.str()});
    if (I.second)
      Worklist.push_back(&*I.first);
  }

Also, is it possible to add a test to exercise the (fixed) the dangling iterator bug?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121533