[lib] Drop 'inline' from 'inline constexpr' variable templates.

[jensmaurer]

Since CWG2387, constexpr variable templates have external linkage.

Fixes #4601

@zygoloid, since const(expr) variable templates now have external linkage, it seems the "inline" is now redundant. What do you think?

[jensmaurer]

@timsong-cpp , should be much better now.

[zygoloid]

@zygoloid, since const(expr) variable templates now have external linkage, it seems the "inline" is now redundant. What do you think?

I agree that removing inline from constexpr variable templates is a correct change. However, if I remember correctly, the LWG folks were informed that the inline is redundant for variable templates (and it was -- arguably -- redundant before CWG2387 too, albeit for a different reason), but they wanted to keep it anyway. Seems worth checking with them.

In passing, I did a pass through the wording to double-check that inline has no other semantic effect, and I found a bug that might be construed as causing problems with this change; here it is with suggested fix:

[module.import]/6: "A header unit shall not contain a definition of a non-inline non-template function or variable whose name has external linkage."

I'm not sure whether we'd consider that editorial, but the intent of the rule is that it only applies to declarations that allow at most one definition.

[tkoeppe]

However, if I remember correctly, the LWG folks were informed that the inline is redundant for variable templates (and it was -- arguably -- redundant before CWG2387 too, albeit for a different reason), but they wanted to keep it anyway. Seems worth checking with them.

Thank you, that's good to know! @jwakely, @CaseyCarter: woudl you happen to have any immediate thoughts on whether the proposed change would be desirable by LWG, or perhaps on the contrary?

[CaseyCarter]

IIRC we were uncomfortable with "arguably redundant before CWG2387" and decided to keep the possibly-redundant inlines until such time as they became unarguably extraneous. I believe that is now the case.

While it wasn't an explicit discussion, I did recently mention this PR on the reflector (https://lists.isocpp.org/lib/2021/06/19716.php) as an argument to avoid adding another redundant inline in an LWG issue resolution, and there was no hue and cry.

[jwakely]

until such time as they became unarguably extraneous

Yes, that was my understanding. "It's probably redundant, but there's an active core issue" wasn't very reassuring 🙂

[jensmaurer]

Editorial meeting 2021-06-18: This is pending the [module.import] fix mentioned by @zygoloid.

[jensmaurer]

@CaseyCarter , please take a look.

@tkoeppe , rebased.

[tkoeppe]

Thanks!

[CaseyCarter]

Everything I see here is good, but I think we're missing:

• exposition-only is-vector-bool-reference in containers.tex (2 places).
• ranges::enable_view and ranges::enable_borrowed_range partial specializations for span in containers.tex.
• disable_sized_sentinel_for partial spec for move_iterator in iterators.tex
• uses_allocator_v primary in memory.tex
• Everything in meta.tex?
• ranges::enable_borrowed_range partials for empty_view, owning_view, as_rvalue_view, reverse_view, zip_view, adjacent_view, chunk_view, slide_view in ranges.tex
• views::istream primary in ranges.tex
• is_bind_expression_v, is_placeholder_v, is_execution_policy_v primary in utilities.tex

[jensmaurer]

@CaseyCarter , thanks, fixed.

[CaseyCarter]

Awesome! Thanks, Jens. One little indentation nit.

[tkoeppe]

All good, all done?

[jensmaurer]

Applied with commit 9ac5555

[tkoeppe]

(I don't understand why this wasn't closed as "merged" when I rebase-squashed it. I thought it might have been a bug in GitHub.)