rewrite_to_chrome_style: associate replacements with the affected file

tools/clang/rewrite_to_chrome_style/EditTracker.cpp

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "EditTracker.h"
+
+#include <assert.h>
+#include <stdio.h>
+#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
+
+void EditTracker::Add(const clang::SourceManager& source_manager,
+                      clang::SourceLocation location,
+                      llvm::StringRef original_text,
+                      llvm::StringRef new_text) {
+  // TODO(dcheng): Can we assume filenames in Chrome are unique? I recall some
+  // bugs in the past when this wasn't true.

[dcheng, 2016/12/22 20:49:40]

I just dump the entire path now; it's easy enough to postprocess components out
if we don't need them.

+  llvm::StringRef filename;
+  for (int i = 0; i < 10; i++) {
+    filename = source_manager.getFilename(location);
+    if (!filename.empty() || !location.isMacroID())
+      break;
+    // Otherwise, no filename and the SourceLocation is a macro ID. Look one
+    // level up the stack...
+    location = source_manager.getImmediateMacroCallerLoc(location);

[Łukasz Anforowicz, 2016/12/22 17:34:36]

I don't understand why we want to do that here.  We only care about
rebase/merge-helping with renames of *full identifiers* - the
rebase/merge-helper will not be able to deal with to##macroArg stuff, right?  If
a file contains rename of a *full identifier* than another replacement will be
present in the EditTracker;  otherwise (a replacement is in the scratch space)
we can just ignore the replacement?

[dcheng, 2016/12/22 20:49:40]

Without this logic to trace upwards, functions that are defined (without token
pasting) inside macros won't be captured in the rebase table.

It's true that we won't correctly handle the token pasting case (in theory we
shouldn't even match against it), but I see that largely as a fringe case?

+  }
+  assert(!filename.empty() && "Can't track edit with no filename!");

[dcheng, 2016/12/22 10:26:22]

For now, I'm going to be more aggressive and assert that this doesn't happen.

I'm not entirely sure this logic is correct... but hopefully it is, it kind of
makes sense that if you're still in a source location which is in scratch space,
you're expanded from some other macro, so you need to go up the macro caller
stack.

[Łukasz Anforowicz, 2016/12/22 17:34:36]

Agreed, but please see me comment above.

+  auto result = tracked_edits_.try_emplace(original_text);
+  if (result.second) {
+    result.first->getValue().new_text = new_text;
+  }
+  result.first->getValue().filenames.try_emplace(
+      llvm::sys::path::filename(filename));
+}
+
+void EditTracker::SerializeTo(llvm::StringRef prefix,
+                              llvm::raw_ostream& output) const {
+  for (const auto& edit : tracked_edits_) {
+    for (const auto& filename : edit.getValue().filenames) {
+      output << filename.getKey() << ":" << prefix << ":" << edit.getKey()
+             << ":" << edit.getValue().new_text << "\n";
+    }
+  }
+}