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

Alignment of declarations in library synopses #2791

Open
CaseyCarter opened this issue Mar 15, 2019 · 4 comments
Open

Alignment of declarations in library synopses #2791

CaseyCarter opened this issue Mar 15, 2019 · 4 comments
Labels
big An issue causing a large set of changes, scattered across most of the text.

Comments

@CaseyCarter
Copy link
Contributor

Declarations in library header and class synopses often go to great lengths to add spaces to align declarations in various ways. We're inconsistent about when we do so, and how we do so. These hand-alignments are fragile, and result in weird "stairsteps" between regions of local consistency. We should establish a consistent policy for which declarations, if any, we align.

Some examples drawn from [utilities]:

  • [utility.syn]:
    template<class T1, class T2>
      constexpr bool operator==(const pair<T1, T2>&, const pair<T1, T2>&);
    template<class T1, class T2>
      constexpr bool operator!=(const pair<T1, T2>&, const pair<T1, T2>&);
    template<class T1, class T2>
      constexpr bool operator< (const pair<T1, T2>&, const pair<T1, T2>&); 
    template<class T1, class T2>
      constexpr bool operator> (const pair<T1, T2>&, const pair<T1, T2>&); 
    template<class T1, class T2>
      constexpr bool operator<=(const pair<T1, T2>&, const pair<T1, T2>&);
    template<class T1, class T2>
      constexpr bool operator>=(const pair<T1, T2>&, const pair<T1, T2>&)
  • [allocator.traits]:
    template<class Alloc> struct allocator_traits {
      using allocator_type     = Alloc;
    
      using value_type         = typename Alloc::value_type;
    
      using pointer            = @\seebelow@;
      using const_pointer      = @\seebelow@;
      using void_pointer       = @\seebelow@;
      using const_void_pointer = @\seebelow@;
    
      using difference_type    = @\seebelow@;
      using size_type          = @\seebelow@;
    
      using propagate_on_container_copy_assignment = @\seebelow@;
      using propagate_on_container_move_assignment = @\seebelow@;
      using propagate_on_container_swap            = @\seebelow@;
      using is_always_equal                        = @\seebelow@;
    
      template<class T> using rebind_alloc = @\seebelow@;   // [ <---- ]
      template<class T> using rebind_traits = allocator_traits<rebind_alloc<T>>;
  • [template.bitset]:
    // 20.9.2.2, bitset operations
    bitset<N>& operator&=(const bitset<N>& rhs) noexcept;
    bitset<N>& operator|=(const bitset<N>& rhs) noexcept;
    bitset<N>& operator^=(const bitset<N>& rhs) noexcept;
    bitset<N>& operator<<=(size_t pos) noexcept;
    bitset<N>& operator>>=(size_t pos) noexcept;
    bitset<N>& set() noexcept;
    bitset<N>& set(size_t pos, bool val = true);
    bitset<N>& reset() noexcept;
    bitset<N>& reset(size_t pos);
    bitset<N>  operator~() const noexcept; // [ <---- ]
    bitset<N>& flip() noexcept;
    bitset<N>& flip(size_t pos);
  • [func.wrap.func]:
    template<class T>       T* target() noexcept;
    template<class T> const T* target() const noexcept;
  • [meta.type.synop]:
    template<class T>
      using remove_const_t    = typename remove_const<T>::type;
    template<class T>
      using remove_volatile_t = typename remove_volatile<T>::type;
    template<class T>
      using remove_cv_t       = typename remove_cv<T>::type;
    template<class T>
      using add_const_t       = typename add_const<T>::type;
    template<class T>
      using add_volatile_t    = typename add_volatile<T>::type;
    template<class T>
      using add_cv_t          = typename add_cv<T>::type;
    [...]
    template<class T>
      inline constexpr bool is_void_v = is_void<T>::value;
    template<class T>
      inline constexpr bool is_null_pointer_v = is_null_pointer<T>::value;
    template<class T>
      inline constexpr bool is_integral_v = is_integral<T>::value;
    template<class T>
      inline constexpr bool is_floating_point_v = is_floating_point<T>::value;
    template<class T>
      inline constexpr bool is_array_v = is_array<T>::value;
    template<class T>
      inline constexpr bool is_pointer_v = is_pointer<T>::value;
@jensmaurer jensmaurer added the decision-required A decision of the editorial group (or the Project Editor) is required. label Mar 15, 2019
@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Jun 5, 2019
@jensmaurer
Copy link
Member

jensmaurer commented Jun 5, 2019

Editorial meeting:

  • Never apply alignment for declarations appearing in different itemdecls.
  • Do align the _v stuff in meta.type.synop; it's easier to read.
  • Rules: Align declarations only if each declaration takes at most 2 lines. Only align on =, return type, and open parentheses for (comparison) operator functions. Make sure aligned blocks of declarations have an empty line before and after.

@tkoeppe
Copy link
Contributor

tkoeppe commented Jun 5, 2019

Thanks! Could you paste this list into the wiki, too?

@tkoeppe
Copy link
Contributor

tkoeppe commented Jun 5, 2019

Some rationale from the discussion: We don't want to ban any sort of alignment outright, nor do we want a hard rule for when to align. Alignment is a tool, and we should use it with good judgement. We agree that it should not be overused, and we have come up with a number of specific constraints and principles:

  • Itemdecls should never use "out-of-context" alignment, i.e. alignment that would only make sense in the context of the corresponding synopsis. (When an itemdecl contains multiple declarations, they may retain alignment that's locally useful.) Moreover, the declaration in a synopsis should be formatted the same as in any itemdecl that repeats it.

  • Alignment is sometimes useful: It can help isolate the declared name amidst a sea of keywords and right-hand-sides, and it can help to emphasize small differences in long passages that share large common subsets.

  • The "Rules" Jens mentions above limit the number of places where we can insert aligning whitespace, so as to keep the number of presentational variations small that readers are confronted with. Moreover, they ensure that any blocks of code that do use alignment are sufficiently separated from ambient code to have local consistency without creating large-scale "blocking" or "staircasing".

  • We considered the historic practice of aligning function names (e.g. "erase" and "insert") excessive and don't condone this in the future. (Cleanup PRs for existing synopses are welcome.)

In summary, within reason we can use alignment where it improves the overall presentation. Do feel free to address specific, focused instances via issues or pull requests where you feel that we can make improvements.

@jensmaurer
Copy link
Member

Rules added to the wiki.

@jensmaurer jensmaurer added the big An issue causing a large set of changes, scattered across most of the text. label Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
big An issue causing a large set of changes, scattered across most of the text.
Projects
None yet
Development

No branches or pull requests

3 participants