klimek added inline comments.
================
Comment at: tools/clang-format/ClangFormat.cpp:277
@@ +276,3 @@
+ std::error_code EC;
+ raw_fd_ostream FileOut(FileName, EC, llvm::sys::fs::F_Text);
+ if (EC) {
----------------
djasper wrote:
> klimek wrote:
> > This is not atomic, which has been a problem in the past (and which is why
> > the rewriter's overwriteChangedFiles is carefully crafted).
> > Any reason to not use a second rewriter? Or even better, can we:
> > 1. Create a rewriter
> > 2. Get affected ranges
> > 3. Iff include sorting, sort includes
> > 4. Recompute affected ranges in new file
> > 5. clang-format
> > 6. Rewrite
> > Alternatively, do not support include sorting and clang-format in the same
> > run and have editor integrations do 2 calls (?)
> Yes, using a second rewriter or recomputing the affected ranges is a
> significant calculation for the sole purpose of un-doing the offsets
> calculated in the rewriter. Furthermore, we already have the new code after
> #include sorting, which we simply calculate again, which seems like wasted
> efforts.
>
> I don't know how the non-atomic-write has been a problem, but if it is, I'd
> prefer to pull out a function or the AtomicallyMovedFile.
>
> I am strongly against letting this be handled by editor integrations so that
> the integrations exhibit consistent behavior and that we don't need to
> implement this N times.
I don't understand yet why we can't use 1 rewriter. The rewriter should
automatically use the updated ranges after we did the fixes, shouldn't it?
I definitely think we'll need to have 2 different runs anyway for all
integrations that run on replacements, or otherwise we'd need to somehow group
replacements in a way that it's clear in which order they need to be applied,
and when the basis for the calculation of the ranges changes.
================
Comment at: unittests/Format/SortIncludesTest.cpp:50-59
@@ +49,12 @@
+
+TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
+ EXPECT_EQ("#include <bbbb.h>\n"
+ "#include <dddd.h>\n"
+ "#include \"aaaa.h\"\n"
+ "#include \"cccc.h\"\n",
+ sort("#include <dddd.h>\n"
+ "#include <bbbb.h>\n"
+ "#include \"cccc.h\"\n"
+ "#include \"aaaa.h\"\n"));
+}
+
----------------
djasper wrote:
> klimek wrote:
> > Do we want to sort C vs C++ system headers?
> Yes, but I want to do refinements to the actual sorting in a follow up. I
> have a bunch of ideas.
SG
http://reviews.llvm.org/D11240
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
