>> --Wno-format-contains-nul -Wno-format-extra-args -Wformat-nonliteral @gol
>> +-Wno-format-contains-nul -Wno-format-extra-args -Wformat-length=1 @gol
>>
>> Most options that take levels are documented as:
>>
>> -Wshift-overflow -Wshift-overflow=@var{n}
>> -Wstrict-overflow -Wstrict-overflow=@var{n}
>
>
> I haven't found any uses of the @var{} tag for numeric arguments
> to options (e.g., -Wformat=2), only for symbolic arguments (e.g.,
> -Warray-bounds=@var{n}), so I'm leaving this as is.

My suggestion was to document -Wformat-length=@var{n}. There is no
difference between -Wformat-length's levels and -Wstrict-overfllow,
-Wshift-overflow, -Wstrict-aliasing levels.
-Wformat=2 is actually the odd one. Note that its description does use:
@item -Wformat
@itemx -Wformat=@var{n}

Even if you want to be explicit and follow -Wformat, then it should be

+-Wno-format-contains-nul -Wno-format-extra-args -Wformat-length
-Wformat-length=2 @gol


> I agree.  The challenge is that not all the bits this depends on
> (the g_string_concat_db and parse_in globals defined in the front
> end) are available in the middle end.  I've been talking to David
> Malcolm about how best to factor things out of c-format.c and make
> it available in both parts of the compiler under a convenient API.

Perhaps diagnostics_context could have pointers to those, forward
defined in the .h file and include the relevant libcpp headers in
diagnostics.c (or input.c). FEs that make use of those features could
initialize them (via some API) to some existing object. Otherwise,
they will work like in your patch (but within diagnostic.c). Similar
to how we initialize the FE-specific pretty-printers.

We already depend on libcpp for line-map.c, so internally depending on
other libcpp features is not so bad. The important thing is to hide
this from the clients, so that the clients do not need to be aware of
what diagnostics.c requires. That is, the middle-end and Ada should
not include headers that include libcpp headers, but diagnostics.c can
include whatever it needs.

Otherwise, the future will be again a mess and we get further away
from ever separating the FEs from the ME.

BTW, it would be nice to explain in comments why each header needs to
be included, besides obvious ones such as tree.h and gimple.h (it
would be great if we had guidelines on how to order included headers,
why not group together all gimple*, tree*, backend-stuff, diagnostics
stuff?). On the other hand, it is unfair to nitpick your patch
regarding this when other commits do the same.

+#include "backend.h"
+#include "tree.h"
+#include "gimple.h"
+#include "tree-pass.h"
+#include "ssa.h"
+#include "gimple-fold.h"
+#include "gimple-pretty-print.h"
+#include "diagnostic-core.h"

Already included in diagnostic.h

+#include "fold-const.h"
+#include "gimple-iterator.h"
+#include "tree-ssa.h"
+#include "tree-object-size.h"
+#include "params.h"
+#include "tree-cfg.h"
+#include "calls.h"
+#include "cfgloop.h"
+#include "intl.h"
+
+#include "builtins.h"
+#include "stor-layout.h"
+
+#include "realmpfr.h"
+#include "target.h"
+#include "targhooks.h"
+
+#include "cpplib.h"

Not in the ME!

+#include "input.h"

already included in coretypes.h

+#include "toplev.h"

Very few files actually need to include toplev.h. Most of them just
include it because of copy-pasting headers lists. And even for bogus
reasons (final.c:#include "toplev.h" /* exact_log2, floor_log2 */)

+#include "substring-locations.h"
+#include "diagnostic.h"

Cheers,

Manuel.

Reply via email to