[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

Awesome, Thank you very much, I dragged my feet on starting to implement something for real.

As a high level comment I think we need to handle requires expressions to get this correct, as `requires requires` would otherwise to bad things.

>From my early brainstorming i got the following options I thought useful. This is in no ways a request for you to implement them, but I would want to get them out here in case they are usefull

  /// Different styles for merging short requires-clauses with the template
    /// decalaration
    enum ShortRequiresClauseStyle {
      /// Never merge requires clauses into a single line.
      /// \code
      ///   template <class T>
      ///   requires someConstraint<T>
      ///   T foo();
      /// \endcode
      SRCS_Never,
      /// Only merge requires clauses with a single constraint.
      /// \code
      ///   template <class T> requires someConstraint<T>
      ///   T foo();
      ///
      ///   template <class T>
      ///   requires someConstraint<T> && otherConstraint<T>
      ///   T bar();
      /// \endcode
      SRCS_Single,
      /// Merge requires clauses if they are short
      /// \code
      ///   template <class T> requires short<T> && shorter<T>
      ///   T foo();
      ///
      ///   template <class T>
      ///   requires someReallyLongElaborateConstraintThatIsReallyLong<T>
      ///   T bar();
      ///
      ///   template <class T>
      ///   requires someLongConstraint<T> && otherLongConstraint<T>
      ///   T baz();
      /// \endcode
      SRCS_Short,
      /// Always try to merge the requires clause with the template declaration
      /// \code
      ///   template <class T> requires someLongConstraint<T> &&
      ///                               otherLongConstraint<T>
      ///   T foo();
      /// \endcode
      SRCS_Always,
    };
  
    /// Dependent on the value, requires-clauses can be put on the same line
    /// as the template declaration.
    ShortRequiresClauseStyle AllowShortRequiresClause;
  
    /// Dependent on the value, requires-expressions can be a single line
    /// Always try to merge the requires clause with the template declaration
    /// \code
    ///   template <class T> requires someLongConstraint<T> &&
    ///                               requires { T{}; }
    ///   T foo();
    /// \endcode
    bool AllowShortRequiresExpression;
  
     /// Dependent on the value break before or after constraint expressions.
    enum ConstraintExpressionBreakingStyle {
      /// Break after constraint expressions but multiple may be on single line
      /// \code
      ///   template <class T> requires someLongConstraint<T> &&
      ///                               otherLongConstraint<T>
      ///   T foo();
      ///
      ///   template <class T> requires short<T> && shorter<T> &&
      ///                               otherLongConstraint<T>
      ///   T bar();
      /// \endcode
      CEBS_After,
      /// Break after constraint expressions.
      /// \code
      ///   template <class T> requires short<T> &&
      ///                               shorter<T> &&
      ///                               otherLongConstraint<T>
      ///   T bar();
      /// \endcode
      CEBS_AfterSingleExpression,
      /// Break before constraint expressions but multiple may be on single line
      /// \code
      ///   template <class T> requires someLongConstraint<T> &&
      ///                               otherLongConstraint<T>
      ///   T foo();
      ///
      ///   template <class T> requires short<T> && shorter<T>
      ///                            && otherLongConstraint<T>
      ///   T bar();
      /// \endcode
      CEBS_Before,
      /// Break before constraint expressions.
      /// \code
      ///   template <class T> requires short<T>
      ///                            && shorter<T>
      ///                            && otherLongConstraint<T>
      ///   T foo();
      /// \endcode
      CEBS_BeforeSingleExpression,
    };
  
    /// The constraint expressions style to use.
    ConstraintExpressionBreakingStyle BreakBeforeConstraintExpression;
  
    /// Dependent on the value wrap before or after requires expressions.
    enum BraceWrappingRequiresExpressionStyle {
      /// Do not wrap braces of requires expressions
      /// \code
      ///   template <class T> requires requires {  T{};
      ///                                           T(int); }
      ///   T foo();
      /// \endcode
      BWARES_NoWrap,
      /// Wrap after requires expressions.
      /// \code
      ///   template <class T> requires requires { T{};
      ///                                          T(int);
      ///                                        }
      ///   T foo();
      /// \endcode
      BWARES_WrapAfter,
      /// Wrap after requires expressions.
      /// \code
      ///   template <class T> requires requires
      ///                               { T{};
      ///                                 T(int); }
      ///   T foo();
      /// \endcode
      BWARES_WrapBefore,
      /// Wrap after requires expressions with a new line.
      /// \code
      ///   template <class T> requires requires
      ///                               {
      ///                                 T{};
      ///                                 T(int); }
      ///   T foo();
      /// \endcode
      BWARES_WrapBeforeWithNewline,
      /// Wrap before requires expressions.
      /// \code
      ///   template <class T> requires requires
      ///                               { T{};
      ///                                 T(int);
      ///                               }
      ///   T foo();
      /// \endcode
      BWARES_WrapBoth,
      /// Wrap before requires expressions.
      /// \code
      ///   template <class T> requires requires
      ///                               {
      ///                                 T{};
      ///                                 T(int);
      ///                               }
      ///   T foo();
      /// \endcode
      BWARES_WrapBothWithNewline,
    };
  
    /// The requires expressions style to use.
    BraceWrappingRequiresExpressionStyle BraceWrappingRequiresExpression;
  
    

My problem was that I had no idea how to properly tokenize the logical-or-constraint-expression. I have pushed my super clumsy WIP stuff here:
https://github.com/miscco/llvm-project/pull/new/clang_format_concepts

If I can help you in any way please tell me



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:1566
       Current.Type = TT_TrailingReturnArrow;
-
+    } else if (Current.is(tok::arrow) && Current.Previous &&
+               Current.Previous->is(tok::r_brace)) {
----------------
Should that really be `tok::arrow` and not `tok::kw_requires`?


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3484
+  // concept ...
+  if (Left.is(TT_TemplateCloser) && Right.is(tok::kw_concept))
+    return true;
----------------
I think we should have an option to allow short requires clauses on the same line similar to allow short functions


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2284
+
+void UnwrappedLineParser::parseRequires() {
+  assert(FormatTok->Tok.is(tok::kw_requires) && "'requires' expected");
----------------
I believe we should separate between constraint expressions aka: `tempalte<  > requires Concept1 && Concept2` and requires expressions aka `requires { ... }`

The two are quite different. That said the handling of requires expressions should be "simple" as we only need to find the matching "}". 

As an upside this would also handle the famous `requires requires`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79773/new/

https://reviews.llvm.org/D79773