r208345 - Remove -Wnon-modular-include

[Richard Smith]

On Thu, May 8, 2014 at 4:39 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Thu, May 8, 2014 at 4:28 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com>wrote:
>
>>
>> On May 8, 2014, at 4:23 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>> On Thu, May 8, 2014 at 4:21 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com>
>> wrote:
>>
>>> Hi Richard,
>>>
>>> On May 8, 2014, at 1:27 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>
>>> On Thu, May 8, 2014 at 1:20 PM, Richard Smith <richard at metafoo.co.uk>
>>> wrote:
>>>
>>>> On Thu, May 8, 2014 at 11:09 AM, Ben Langmuir <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.)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140508/241719b7/attachment.html>