Page MenuHomePhabricator

[clang-format] [PR45198] deletes whitespace inside of a noexcept specifier
Needs ReviewPublic

Authored by MyDeveloperDay on May 15 2020, 3:12 PM.

Details

Summary

https://bugs.llvm.org/show_bug.cgi?id=45198

The following:

template<
    typename T,
    enable_if_t<is_move_assignable<T>::value> = true,
    enable_if_t<is_move_constructible<T>::value> = true>
constexpr void
swap(T &lhs, T &rhs) noexcept(
    is_nothrow_move_constructible<T>::value && is_nothrow_move_assignable<T>::value)

Results in this:

template<
    typename T,
    enable_if_t<is_move_assignable<T>::value> = true,
    enable_if_t<is_move_constructible<T>::value> = true>
constexpr void
swap(T &lhs, T &rhs) noexcept(
    is_nothrow_move_constructible<T>::value &&is_nothrow_move_assignable<T>::value)

This is because the && in is_nothrow_move_constructible<T>::value &&is_nothrow_move_assignable<T>::value gets incorrectly determined to be a TT_PointerOrReference

This revision attempts to detect determine a cases where this cannot be true especially in a noexcept context where the result is expected to be boolean

Diff Detail

Event Timeline

MyDeveloperDay created this revision.May 15 2020, 3:12 PM

I mainly have concerns with the search for noexcept being too brittle and not looking far enough back. Implementing that may have performance implications though, so I have no issue ignoring that problem if it's proven sufficiently rare.

Also: should this do the same thing for throw()? It has pretty much the same constraints/context and would be trivial to add, although it's deprecated and/or removed in recent C++ versions. I'm not particularly inclined to implement that, but the question ought to be raised since it theoretically has the same problem.

clang/lib/Format/TokenAnnotator.cpp
1933

Doesn't this not handle nested templates? For example, noexcept(Foo<Bar<T>>::value && Baz) would not work here. Ditto on expressions like Foo<T1, T2>.

I'm having trouble thinking of a clean generic solution, but the most robust thing I can think of is to search the current line for the previous opening parens and check if noexcept is before it (if they use multiple sets of parens, the other heuristics seem to cover it).