[PATCH] Change OwningPtr::take() to OwningPtr::release().

[Ahmed Charles]

  Thanks.


================
Comment at: include/clang/Frontend/FrontendAction.h:149
@@ -148,5 +148,3 @@
 
-  ASTUnit *takeCurrentASTUnit() {
-    return CurrentASTUnit.take();
-  }
+  ASTUnit *takeCurrentASTUnit() { return CurrentASTUnit.release(); }
 
----------------
David Blaikie wrote:
> I think clang-format might now have a mode in which it doesn't reformat lines you didn't edit (so this would remain on 3 lines instead of being clumped into one)... 
> 
> But I'm not too fussed - just a possible FYI for this or next time in case you endu p dealing with more complicated patches that would benefit from easier diffs.
I don't see that as a command-line flag. It does refuse to make changes to certain things which are on lines that weren't edited, but it also does things like this. In any case, I went through each edit and decided they were fine given the complexity of the actual change going on and hopefully it will make future changes have nicer diffs.

================
Comment at: lib/CodeGen/ModuleBuilder.cpp:51
@@ -50,5 +50,3 @@
 
-    virtual llvm::Module* ReleaseModule() {
-      return M.take();
-    }
+    virtual llvm::Module *ReleaseModule() { return M.release(); }
 
----------------
David Blaikie wrote:
> Feel free to commit cleanup (like the moved '*' or the move to one-line - if you aren't using the "only modify lines I touched" thing I mentioned earlier - or if I hallucinated and that doesn't really exist) ahead of time without precommit review if you like - though it's not a big deal on a patch like this that's been manufactured by replacements and clang-format.
Interestingly enough, the first diff is without clang-format running and the second is after it was run, so you could always just look at the first one, which would have the same effect, at least for pre-review purposes.

But agreed, that would be nicer for more complex changes.

================
Comment at: lib/Lex/PTHLexer.cpp:539
@@ -540,1 +538,3 @@
+                        SL.release(), NumIds, spellingBase,
+                        (const char *)originalSourceBase);
 }
----------------
David Blaikie wrote:
> More orthogonal cleanup - might be worth filing bugs on clang-format-patch (or git-clang-format, or whatever other tools) for these.
I'm not sure whether it's a bug or by design. The vim integration code for clang-format says: The line or region is extended to the next bigger syntactic entity.

This suggests that this is possibly a feature. I'm not sure what it would mean to reflow the first two lines without ever changing the third in this scenario, for example, if adding release make it hit the 80-col limit twice, it would obviously be better to change the third line rather than insert a new one. But in making that work correctly, you end up with orthogonal changes happening as well.

In any case, it seems like clang-format isn't designed to simply ignore existing formatting, for example, there's no option to have it not touch the following code:

`#include <foo> // comment`
`#include <barbarbar> // comment`
`#include <baz>       // comment`

It will either change the indentation of the comment on line 1 or line 3, but no matter what you do, it will change one of them.


http://llvm-reviews.chandlerc.com/D3000