rewrite_to_chrome_style: associate replacements with the affected file

tools/clang/rewrite_to_chrome_style/EditTracker.h

+// 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.
+
+#ifndef TOOLS_CLANG_REWRITE_TO_CHROME_STYLE_EDIT_TRACKER_H_
+#define TOOLS_CLANG_REWRITE_TO_CHROME_STYLE_EDIT_TRACKER_H_
+
+#include <map>
+
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"

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

very nitty nit / please ignore me: Can me forward declared?

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

It could be. But I only bothered forward declaring the things LLVM style
specifically asks not to include in headers (i.e. raw_ostream)

(Current Google style encourages includes over forward decls, though Chromium
itself doesn't follow this guideline)

+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+
+namespace llvm {
+class raw_ostream;
+}  // namespace llvm
+
+struct EditInfo {
+  std::string new_text;
+  llvm::StringSet<> filenames;
+};
+
+// Simple class that tracks the edits made by path. Used to dump the databaes
+// used by the Blink rebase helper.
+class EditTracker {
+ public:
+  EditTracker() = default;
+
+  void Add(const clang::SourceManager& source_manager,
+           clang::SourceLocation location,
+           llvm::StringRef original_text,
+           llvm::StringRef new_text);
+
+  // Serializes the tracked edits to |output|. Emits:
+  // <filename>:<prefix>:<original text>:<next text>

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

typo:s/next/new/

nit/suggestion: s/<prefix>/<tag>/
(+ change the name of the parameter from |prefix| to |tag|?)

For a moment I wanted to suggest line/delimiter format consistent with the
format of edits, but then I realized that <original text> and <new test> should
be *identifiers* and so should never include a ":" character.  OTOH, if we're
going to do something similar in the future for non-identifiers, then maybe it
makes sense to start?  But than YAGNI... I am not sure what do do here.

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

We already have a hack for newlines:
https://cs.chromium.org/chromium/src/tools/clang/scripts/run_tool.py?rcl=0&l=108

If it comes down to it, we can extend it for : as well.

Done with the rename suggestions.

+  // for each distinct filename for each tracked edit.
+  void SerializeTo(llvm::StringRef prefix, llvm::raw_ostream& output) const;
+
+ private:
+  EditTracker(const EditTracker&) = delete;

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

Is this meant to be an equivalent of DISALLOW_COPY_*AND_ASSIGN*?  I see that
base/macros.h also deletes the assignment operator.

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

Done.

+
+  // The string key is the original text.
+  llvm::StringMap<EditInfo> tracked_edits_;

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

Incidentally, LLVM solved the whole "std::map<std::string, ...>" is sad
situation by just have a very specific implementation.

Perhaps we should just do that in Chrome...

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

I don't know what is meant by "sad situation" + I understand that this is not
really related to this CL, but - isn't there a proposal for a future version of
C++ stdlib that will allow map lookups by key to use alternative types?  (i.e.
allowing key lookup using |const char*| value, without requiring allocation of a
separate/temporary |std::string|)  If this is the case (and *this* is the
sadness) then maybe we don't want to introduce a Chromium-specific type but
rather wait until the standard gets less sad?

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

It doesn't help with unordered sets, and the transparent comparator is a bit
awkward to use.

+};
+
+#endif  // #define TOOLS_CLANG_REWRITE_TO_CHROME_STYLE_EDIT_TRACKER_H_