Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

clang-format Require Clause fails to break correctly when using parenthesis. #51743

Closed
Sebanisu opened this issue Nov 4, 2021 · 7 comments
Closed

clang-format Require Clause fails to break correctly when using parenthesis. #51743

Sebanisu opened this issue Nov 4, 2021 · 7 comments
Assignees
@HazardyKnusperkeks
Labels
bugzilla Issues migrated from bugzilla clang-format

Comments

@Sebanisu
Copy link

Sebanisu commented Nov 4, 2021 •
edited by mkurdej

Bugzilla Link 52401
Version 13.0
OS Windows NT
Attachments my clang format file
CC @HazardyKnusperkeks,@zygoloid

Extended Description

related to: #31513
It seems to have some basic formatting that works okay as long as you don't use parenthesis. If I wrap the Requires clause in parenthesis the whole thing and the following line will be combined into one line. At least in some of my testing.
I'm using the ClangFormat built into Clion it appears to be ClangFormat 13.

//start without parenthesis
 template<
    bool nested = true,
    typename lambdaT,
    typename filterT = decltype(default_filter_lambda)>
  requires valid_execute_on_lambda<lambdaT> && valid_filter_lambda<filterT>
  bool
    execute_on(
      const std::initializer_list<std::string_view> &filename,
      lambdaT                                      &&lambda,
      filterT                                      &&filter_lambda = {}) const
  {
    return loop(get_execute_on_lambda<nested>(filename, lambda, filter_lambda));
  }
//end without.

//start with
template<
    bool nested = true,
    typename lambdaT,
    typename filterT = decltype(default_filter_lambda)>
  requires(valid_execute_on_lambda<lambdaT> &&valid_filter_lambda<filterT>) bool execute_on(
    const std::initializer_list<std::string_view> &filename,
    lambdaT                                      &&lambda,
    filterT                                      &&filter_lambda = {}) const
  {
    return loop(get_execute_on_lambda<nested>(filename, lambda, filter_lambda));
  }
//end with

The one below here requires the parathesis because it has two conditions. See how the function type and function name are forced to be on the same line as the last condition?

//start another example with two groups of parenthesis
 template<
    bool nested = true,
    ArchiveTypeT... aT,
    typename lambdaT,
    typename filterT = decltype(default_filter_lambda)>
  requires(valid_execute_on_lambda<lambdaT> &&valid_filter_lambda<filterT>)
    && (sizeof...(aT) > 0) bool execute_on(
      const std::initializer_list<std::string_view> &filename,
      lambdaT                                      &&lambda,
      filterT                                      &&filter_lambda = {}) const
  {
    return specify<aT...>(
      get_execute_on_lambda<nested>(filename, lambda, filter_lambda));
  }
};
//end of another example.
@Sebanisu
Copy link
Author

Sebanisu commented Nov 4, 2021

assigned to @HazardyKnusperkeks

@Sebanisu
Copy link
Author

Sebanisu commented Nov 4, 2021 •
edited by mkurdej

I noticed this website is wrapping the text for us. Maybe if you view source you'll see it isn't wrapping.
I took away the first group of parentheses and only have the second set. It put the whole requires clause and the function type and function name in the same line.

//begin another variation of this issue
  template<
    bool nested = true,
    ArchiveTypeT... aT,
    typename lambdaT,
    typename filterT = decltype(default_filter_lambda)>
  requires valid_execute_on_lambda<lambdaT> && valid_filter_lambda<filterT> &&(sizeof...(aT) > 0) bool execute_on(
    const std::initializer_list<std::string_view> &filename,
    lambdaT                                      &&lambda,
    filterT                                      &&filter_lambda = {}) const
  {
    return specify<aT...>(
      get_execute_on_lambda<nested>(filename, lambda, filter_lambda));
  }

@Sebanisu
Copy link
Author

Sebanisu commented Nov 4, 2021 •
edited by mkurdej

I put the sizeof... first and it wrapped on the && though it seemed confused till I removed the (sizeof...(aT) > 0) altogether. Still didn't break correctly before the function type.

//begin other version
template<
    bool nested = true,
    ArchiveTypeT... aT,
    typename lambdaT,
    typename filterT = decltype(default_filter_lambda)>
  requires(sizeof...(aT) > 0)
    && valid_execute_on_lambda<lambdaT>
      &&valid_filter_lambda<filterT> bool execute_on(
        const std::initializer_list<std::string_view> &filename,
        lambdaT                                      &&lambda,
        filterT                                      &&filter_lambda = {}) const
  {
    return specify<aT...>(
      get_execute_on_lambda<nested>(filename, lambda, filter_lambda));
  }
};
//end other version

//workaround
  template<ArchiveTypeT... aT>
  static constexpr bool not_zero = sizeof...(aT);
//start workaround result
  template<
    bool nested = true,
    ArchiveTypeT... aT,
    typename lambdaT,
    typename filterT = decltype(default_filter_lambda)>
  requires not_zero<aT...> && valid_execute_on_lambda<
    lambdaT> && valid_filter_lambda<filterT>
  bool
    execute_on(
      const std::initializer_list<std::string_view> &filename,
      lambdaT                                      &&lambda,
      filterT                                      &&filter_lambda = {}) const
  {
    return specify<aT...>(
      get_execute_on_lambda<nested>(filename, lambda, filter_lambda));
  }
//end workaround

@Sebanisu
Copy link
Author

Sebanisu commented Nov 4, 2021 •
edited by mkurdej

workaround in previous example should of been this I forgot the > 0U though it would probably still work with out.

//workaround fixed
  template<ArchiveTypeT... aT>
  static constexpr bool not_zero = sizeof...(aT) > 0U;

@HazardyKnusperkeks
Copy link
Contributor

I'm working on it.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
@HazardyKnusperkeks HazardyKnusperkeks added the awaiting-review Has pending Phabricator review label Feb 5, 2022
@HazardyKnusperkeks
Copy link
Contributor

Review link: https://reviews.llvm.org/D113319

@HazardyKnusperkeks
Copy link
Contributor

Pushed

@github-actions github-actions bot removed the awaiting-review Has pending Phabricator review label Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@HazardyKnusperkeks HazardyKnusperkeks

Labels
bugzilla Issues migrated from bugzilla clang-format
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Sebanisu @HazardyKnusperkeks