[cfe-dev] Challenging "-Wctad-maybe-unsupported" for 9.0.0

[JVApen via cfe-dev]

On Sun, 4 Aug 2019 at 00:37, Eric Fiselier <eric at efcs.ca> wrote:

>
>
> On Sat, Aug 3, 2019 at 10:14 AM Arthur O'Dwyer via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> On Sat, Aug 3, 2019 at 6:03 AM JVApen via cfe-dev <cfe-dev at lists.llvm.org>
>> wrote:
>>
>>> Hello all,
>>>
>>> I'm currently testing 9.0.0 and I'm one of those strange people that
>>> uses -Weverything in production, as we only support a single clang-release
>>> at the same time. (Actually clang-cl, as it's Windows-only)
>>> During the upgrade, I've noticed a new warning
>>> "-Wctad-maybe-unsupported".
>>> At first, I was really happy seeing it and started updating some code
>>> that implicitly used it.
>>> I'm even considering to propose an extension to our styleguide, as
>>> mentioned in the review of the warning: https://reviews.llvm.org/D56731
>>> `Some style guides want to allow using CTAD only on types that "opt-in"`
>>>
>>> However, after finishing my initial testing and applying some
>>> suppression and update locally, I'm changing my mind.
>>> Over half of the updates I had to do were about `std::unique_lock`,
>>> `std::scope_guard` ... (and this without actively using this feature).
>>>
>>
> What do you mean by "without actively using this feature"?
>

We presented the use of C++17 when we enabled it, we never mentioned CTAD,
so the only users of it are either the few programmers who actually know
about it or accidental usages.


> It is unfortunate that this warning will flag types designed to work with
> CTAD, but who don't need any explicit deduction guides.
> Over time, I hope libraries that care about CTAD will address this by
> suppressing the warning manually; but that's not an ideal solution.
>
>
>
>> These classes, if I'm correct, are one of the selling point of CTAD and
>>> are prohibited thanks to this warning.
>>>
>>
> They're not meant to be triggered by this warning. It's just an oversight.
>
> I'll update and audit libc++ to opt-in where needed. I'll also set up
> libc++'s test suite to build with this warning
> enabled.
>
> I'll try to get the fixed into 9.0.
>

I wasn't very explicit about this, however, we only use clang-cl with the
MSVC headers. So from my point of view, it ain't that urgent. Although it
technically being undefined behavior, we can write such a header as well
and include it in our precompiled header (/FI
<https://docs.microsoft.com/en-us/cpp/build/reference/fi-name-forced-include-file?view=vs-2019>
).
Just wondering, is it allowed for the standard library implementation to
add these deduction guides without them being specified in the standard? (I
presume yes, as behavior doesn't change and one can't detect if this
deduction guide exists)


>
>
>>
>>> To circle back to title of the email, should -Wctad-maybe-unsupported
>>> actually be a compiler warning iso a clang-tidy check?
>>>
>>
>> (You mean "should -Wctad-maybe-unsupported be a clang-tidy check instead
>> of a compiler warning?", I assume.)
>>
> Yes, indeed. My bad.

>
>>
>>> If so, would it make sense of having a white-list, like all `std::`
>>> classes?
>>>
>>
> The diagnostic is no less valid for types in the standard library. The
> library author should audit the type to ensure it's CTAD safe, write tests,
> and then explicitly opt it in. This is
> important for the correctness of client programs written using CTAD.
>
>
>> FWIW, I continue to believe that it makes no sense to talk about the *library
>> author* "opting in" to CTAD.
>>
>
> Types have to be designed with CTAD in mine. Without explicit deduction
> guides CTAD often does the wrong thing.
> Even otherwise unobservable qualities about how a constructor is lexically
> spelled in source can greatly impact CTAD.
> Whether a template was designed to support CTAD is an important quality we
> should talk more about.
>
>
>
>> Either the *client programmer* trusts CTAD (even if it runs the risk of
>> deducing the wrong thing sometimes) and thus every
>> set-of-missing-angle-brackets must be assumed to be intentional no matter
>> what type it's on, or else the client programmer doesn't trust CTAD
>> (because of the aforementioned risk), and thus every
>> set-of-missing-angle-brackets must be assumed to be unintentional.
>>
>
> This isn't a dichotomy.
>
> To be clear, "-Wctad-maybe-unsupported" diagnoses only usages of CTAD on
> types without any explicit deduction guides.
>
>
>
>> The intentions of the *library author* don't matter at all in this
>> equation.
>>
>
>

> Whether a type was designed with knowledge of CTAD in mind has a causal
> relationship with the correctness of usages of that type with CTAD.
> As client programmer I don't want to use CTAD on types that haven't been
> changed since before CTAD was a feature.
>
Agreed!


>
>
>> The situation for me is exactly analogous to VLAs in C — including the
>> shape of the workaround: "write an explicit call to malloc()" versus "write
>> an explicit call to make_foo()".
>> https://reviews.llvm.org/D54565 implements a `-Wctad` warning analogous
>> to `-Wvla`.
>>
>
I'm not sure we are talking about the same thing here, from what I read in
that review, it flags all usages of CTAD. While the warning I mentioned
doesn't trigger if the deduction guide exists. Although both use-cases are
relevant, I'm in the luxurious position of having a single C++ standard to
work with (and worry about).

–Arthur
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190804/26551a45/attachment.html>