[PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

[strager via cfe-commits]

strager added a comment.

(Sorry for the late feedback.)


================
Comment at: docs/ClangFormatStyleOptions.rst:221
@@ -220,2 +220,3 @@
 
-**AlwaysBreakAfterDefinitionReturnType** (``DefinitionReturnTypeBreakingStyle``)
+**AlwaysBreakAfterDeclarationReturnType** (``ReturnTypeBreakingStyle``)
+  The function declaration return type breaking style to use.
----------------
djasper wrote:
> Do you think it'll ever make sense to break differently for declarations and definitions? I think having two entirely independent configuration flags gives us 9 combinations (assuming the three enum values will remain) out of which many will never be used.
> 
> I see two alternatives:
> Add an additional bool flag "TreatDeclarationsLikeDefinitions" (name might not be ideal yet).
> Change existing flag name to AlwaysBreakAfterReturnType and use five enum values (None, TopLevel, All, TopLevelDefinitions, AllDefinitions).
> 
> What do you think?
> 
> Sorry for being very picky on this. The problem is that these options stick around for a long time and we need to think about them carefully.
> I think having two entirely independent configuration flags gives us 9 combinations (assuming the three enum values will remain) out of which many will never be used.

I agree that many combinations will be unused. I don't see why we should conflate the two if they are configured independently in practice, though.

> Add an additional bool flag "TreatDeclarationsLikeDefinitions" (name might not be ideal yet).
> Change existing flag name to AlwaysBreakAfterReturnType and use five enum values (None, TopLevel, All, TopLevelDefinitions, AllDefinitions).

What about styles where declarations have return types on their own line, but definitions don't? I don't see a reason to prevent that style.

================
Comment at: lib/Format/ContinuationIndenter.cpp:129-132
@@ -128,5 +128,6 @@
   if (Current.is(TT_FunctionDeclarationName) &&
       Style.AlwaysBreakAfterDefinitionReturnType == FormatStyle::DRTBS_None &&
+      Style.AlwaysBreakAfterDeclarationReturnType == FormatStyle::DRTBS_None &&
       State.Column < 6)
     return false;
 
----------------
djasper wrote:
> What do you mean exactly?
See `IsDefinition` in `lib/Format/TokenAnnotator.cpp` introduced by this diff. The logic here and there should match, I think.


http://reviews.llvm.org/D10370