> What is the cost of updating range info every transform? I have a feeling
this new range management stuff is more complex than it needs to be. It's not
done right now but the plan was to store replacements for headers to write them
to disk at the end. We could do something similar for sources too and then we
could just do the range calculation once at the end.
I believe your are referring to the cost of calling
`SourceOverrides::collectReplacementData(const Replacements &Replaces)` every
time a transform is done? Honestly I don't know how to calculate the cost (are
you asking for a Big O notation?).
It's sure kind of costly, lots of iterations and some inner loops but I did
it because it's certainly less costly than reformatting after every transforms.
I'm curious about how replacements' serialization will work, if it's done in
a way that at the end we have only one list of replacements to apply then I
agree the reformatting can be done in a much more efficient way. In the
meantime it seemed to be a reasonable way to implement it.
================
Comment at: cpp11-migrate/Core/FileOverrides.h:37
@@ +36,3 @@
+///
+// FIXME: clang::tooling::Range could be used if they were a bit more flexible
+// (e.g: offset and length reassignable)
----------------
Edwin Vane wrote:
> Is there a reason we can't improve clang::tooling::range?
I saw the class quite lately when everything was already working. I will try to
get the functionality I need to be committed in `tooling::Range`.
================
Comment at: unittests/cpp11-migrate/FileOverridesTest.cpp:21
@@ -19,18 +20,3 @@
-TEST(SourceOverridesTest, Interface) {
- llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> DiagOpts(
- new DiagnosticOptions());
- DiagnosticsEngine Diagnostics(
- llvm::IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs()),
- DiagOpts.getPtr());
- FileManager Files((FileSystemOptions()));
- SourceManager SM(Diagnostics, Files);
- StringRef FileName = "<text>";
- StringRef Code =
- "std::vector<such_a_long_name_for_a_type>::const_iterator long_type =\n"
- " vec.begin();\n"
- "int x;"; // to test that it's not the whole file that is reformatted
- llvm::MemoryBuffer *Buf = llvm::MemoryBuffer::getMemBuffer(Code, FileName);
- const clang::FileEntry *Entry =
- Files.getVirtualFile(FileName, Buf->getBufferSize(), 0);
- SM.overrideFileContents(Entry, Buf);
+// Test fixture object that setup some files once for all test cases and remove
+// them when the tests are done.
----------------
Edwin Vane wrote:
> These refactoring changes are not specific to the reformat stuff. Can you
> separate them into another patch?
Sure.
http://llvm-reviews.chandlerc.com/D1136
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
