r207975 - Add -Wmodule-build to make it easy to see when modules are (re)built

[Richard Smith]

On Thu, May 8, 2014 at 3:48 PM, Ben Langmuir <blangmuir at apple.com> wrote:

>
> On May 7, 2014, at 6:01 PM, Ben Langmuir <blangmuir at apple.com> wrote:
>
>
> On May 7, 2014, at 8:46 AM, Tobias Grosser <tobias at grosser.es> wrote:
>
> On 06/05/2014 23:13, Ben Langmuir wrote:
>
>
> On May 6, 2014, at 12:48 AM, Tobias Grosser <tobias at grosser.es> wrote:
>
> On 05/05/2014 22:07, Ben Langmuir wrote:
>
>
> On May 5, 2014, at 12:54 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Mon, May 5, 2014 at 12:27 PM, Ben Langmuir <blangmuir at apple.com> wrote:
> Hi Richard,
>
> I agree in principle, but I was under the impression remark wasn’t fully
> baked for clang diagnostics yet.  For example, the commit message says:
>
> This patch is by intention minimal in terms of parameter handling. More
> experience and more discussions will most likely lead to further
> enhancements
> in the parameter handling.
>
>
>
> And indeed, I gave it a spin and immediately noticed that it prints out [
> -Rmodule-build ] in the diagnostics, which is actively misleading when -R
> is not a supported diagnostic option spelling.
>
> Yeah, we don't support the command-line interface for it yet, but that
> should be straightforward. That burden has basically been deferred to the
> first "lucky" person who wants to add a remark. Looks like that might be
> you? :)
>
>
> I did not want to make the impression to avoid the work here.
>
> Adding Tobias who wrote the commit message I’m interpreting :)
>
>
> Thanks.
>
> I took “more experience and more discussions” to mean we didn’t know what
> we wanted yet.  If my interpretation is incorrect, and it really is just a
> matter of wiring up -R like a simplified* -W, then I agree that it should
> be straightforward to implement
>
>
> Yes, your interpretation is right. At that point, the idea of remarks
> themselves was rather clear and already tested in Polly, but for the
> command line interface there where different proposals that where mostly
> discussed in the white. The idea was to get the basic infrastructure in
> place and then use discussions around upcoming remarks (vectorizer and
> inliner were the most likely ones) to drive the design of the command lines.
>
> Almost immediately after I committed these patches, Diego stepped up to
> work on the inliner and vectorizer remarks and for those the
> command line options -Rpass=passname e.g., -Rpass=inline have been
> chosen. With your change, we now seem to have another complimentary
> use-case for remarks. That is great.
>
> * By simplified, I mean that I assume we don’t want all of the special
> case -W options like everything, error, system-headers ...
>
>
> From what I learned from the previous remark discussions I think the a
> simplified -R option is really the way to go. Am I right, that for
> your remark, using a flag -Rmodule-build would be what you would
> like to use?
>
>
> I don’t really have an opinion about what the spelling ought to be.
>  -Rmodule-build would be fine with me, but so is -Wmodule-build if we were
> consistent.
>
>
> OK. Then let's go for -Rmodule-build. That seems better in line to what
> Diego introduced. Would you like to give it a shot?
>
>
> Not particularly ;-)  It would also be a while before I would have time to
> look at this.
>
>
> On reflection it seemed prudent to turn ‘module-build’ into a remark even
> though the spelling will still be -Wmodule-build for now (done in r208367).
>
> This raised another question: should -Weverything turn on remarks, or not?
> I’m on the fence.
>

I don't think it should: -Weverything is for people who want "tell me
anything that might possibly be wrong here", and remarks are not indicating
a potential problem. I also think the -W and -R flags should be
independent, so even if we want an 'all remarks' mode, it should be called
-Reverything.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140508/cd152817/attachment.html>