r208345 - Remove -Wnon-modular-include
Argyrios Kyrtzidis
akyrtzi at gmail.com
Thu May 8 20:43:50 PDT 2014
On May 8, 2014, at 5:31 PM, Alp Toker <alp at nuanti.com> wrote:
>
> On 09/05/2014 01:08, Argyrios Kyrtzidis wrote:
>>
>> On May 8, 2014, at 4:53 PM, Richard Smith <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>> wrote:
>>
>>> On Thu, May 8, 2014 at 4:39 PM, Richard Smith <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>> wrote:
>>>
>>> On Thu, May 8, 2014 at 4:28 PM, Argyrios Kyrtzidis
>>> <akyrtzi at gmail.com <mailto:akyrtzi at gmail.com>> wrote:
>>>
>>>
>>> On May 8, 2014, at 4:23 PM, Richard Smith
>>> <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>> wrote:
>>>
>>>> On Thu, May 8, 2014 at 4:21 PM, Argyrios
>>>> Kyrtzidis<akyrtzi at gmail.com <mailto:akyrtzi at gmail.com>>wrote:
>>>>
>>>> Hi Richard,
>>>>
>>>> On May 8, 2014, at 1:27 PM, Richard Smith
>>>> <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>>
>>>> wrote:
>>>>
>>>>> On Thu, May 8, 2014 at 1:20 PM, Richard
>>>>> Smith<richard at metafoo.co.uk
>>>>> <mailto:richard at metafoo.co.uk>>wrote:
>>>>>
>>>>> On Thu, May 8, 2014 at 11:09 AM, Ben
>>>>> Langmuir<blangmuir at apple.com
>>>>> <mailto:blangmuir at apple.com>>wrote:
>>>>>
>>>>> Author: benlangmuir
>>>>> Date: Thu May 8 13:09:29 2014
>>>>> New Revision: 208345
>>>>>
>>>>> URL:http://llvm.org/viewvc/llvm-project?rev=208345&view=rev
>>>>> Log:
>>>>> Remove -Wnon-modular-include
>>>>>
>>>>> But keep
>>>>> -Wnon-modular-include-in-[framework-]module
>>>>>
>>>>> This warning is too noisy and doesn't really
>>>>> indicate a problem for most
>>>>> people. Even though it would only really affect
>>>>> people using
>>>>> -Weverything, that seems bad so remove it.
>>>>>
>>>>>
>>>>> Wait a second, we were planning on using this to
>>>>> replace -fmodules-strict-decluse. People using
>>>>> -Weverything need to use -Wno-foo for the warnings
>>>>> they don't want; that's the cost of using that option.
>>>>>
>>>>>
>>>>> From when -Weverything was added:
>>>>>
>>>>> Eli Friedman wrote:
>>>>> > This seems like a bad idea: people will start using
>>>>> it, then complain
>>>>> > whenever we add a new warning which isn't generically
>>>>> applicable
>>>>> > (suppose we add an opt-in warning for C-style casts
>>>>> in C++, for
>>>>> > example). We already have -Wextra for people who want
>>>>> lots of> warnings.
>>>>>
>>>>> Ted Kremenek responded:
>>>>> [...]
>>>>> > -Weverything is not for everybody, and for those that
>>>>> complain about
>>>>> > new warnings we can (and should) tell them to take
>>>>> the opt-in approach
>>>>> > as opposed to the opt-out approach. I don't see a
>>>>> problem with really
>>>>> > serving both sets of user preferences.
>>>>>
>>>>> => We do not get to use -Weverything as an excuse for
>>>>> removing warning that are noisy and that some users
>>>>> aren't interested in.
>>>>
>>>> This is not just noisy, it has no value IMO. How can we
>>>> expect an application not to #include something
>>>> textually and what is the user supposed to do when they
>>>> see this warning ?
>>>>
>>>>
>>>> If the application intends to have all of its own headers
>>>> modularized, this warning can be used to check that. If the
>>>> user sees this warning, and they're in that case, they
>>>> should either add the header to a module or explicitly
>>>> exclude it (if it's supposed to be textually included).
>>>
>>> I don’t think this is feasible in practice, preprocessor
>>> tricks and textual includes will not go away, they have real
>>> uses.
>>>
>>>
>>> They do, but you can exclude headers meant for textual inclusion
>>> just as easily for an application as for a library.
>>>
>>> The warning is very important for the framework/library
>>> author, but I don’t think it provides value in practice for
>>> the application author.
>>>
>>>
>>> I disagree. An application may want to use modules for itself.
>>> They're not limited to libraries.
>>>
>>>
>>> I've done some more digging and I think what we want to replace -fmodules-strict-decluse is to warn if either
>>> a) a module build includes a header that's not referenced by any module map, or
>>> b) a non-module build with -fmodule-name= specified references such a header
>>> ... which looks like it matches the cases that the remaining -Wnon-modular-include-in-module catches, so that's a non-issue.
>>>
>>> However, for my work modularizing LLVM, the -Wnon-modular-include warning has *already* been very useful, despite having only been available for a few hours. I have first-hand experience that this *is* useful functionality for application authors. That said, I think we should suppress the warning if the includer isn't in the main source file (I don't care that my non-modular includes have transitive non-modular includes.)
>>
>> How about adding it as a remark ? That way you can get the same benefit as before without the negative connotation that the user is doing something “wrong” by simply #including something.
>
> I'd be cautious of categorising this at *lower* severity than, say, -Wold-style-cast, because the distinction appears arbitrary. Are C-style casts more severe than non-modular includes? How long is a piece of string?
FWIW, I'm not fond of -Wold-style-cast. I subscribe to the notion that compiler warnings should point out potential problems in the code, not enforce style; style should be the domain of another tool.
>
> The existing set of off-by-default warnings would seem fine for this, or perhaps clang-tidy.
>
> Alp.
>
> --
> http://www.nuanti.com
> the browser experts
>
More information about the cfe-commits
mailing list