On 27 November 2014 at 20:28, Tobias Burnus <bur...@net-b.de> wrote:
> I think the approach is fine. As the _now version overrides the buffer, one
> might even do with a single buffer by clearing it, setting flush_p
> temporarily to true and printing the message. It only might collide with
> buffered warnings (for error_now) and errors (for warning_now), but I don't
> see whether that's an issue. For warnings/error_now it probably isn't, for
> errors/warning_now, it might. Thus, having two buffers is probably better.
Oh, I didn't notice that the _now versions override the buffered
messages. Where do you see that? It seems that gfc_warning_1 saves and
restores buffer_flag but does not touch the buffers (it prints
directly to the stream).
However, as you say, I may still need two buffers, one for errors and
other for warnings. (If this were not necessary, it would be great! Is
it really?).
In fact, we will need more buffers, since there is gfc_push/pop_error.
This is why I didn't start with gfc_error, it is a bit more complex
than gfc_warning.
>> The ugliest part is how to handle warningcount and werrorcount. I
>> could handle this in the common machinery in a better way by storing
>> DK_WERROR in the diagnostic->kind and checking it after printing.
>
>
> I'm missing the "but". Does it require more modifications in common code and
> thus you shy away? Or is there another reason?
I was going to write "Exactly" and a long explanation. But now I
realize that in this particular case I can simply check DK_ERROR,
since within gfc_warning that can only come from a warning converted
to an error. That simplifies the function a bit:
+/* Issue a warning. */
+
+bool
+gfc_warning_1 (int opt, const char *gmsgid, ...)
+{
+ va_list argp;
+ diagnostic_info diagnostic;
+ bool fatal_errors = global_dc->fatal_errors;
+ pretty_printer *pp = global_dc->printer;
+ output_buffer *tmp_buffer = pp->buffer;
+
+ if (!pp_warning_buffer.flush_p)
+ {
+ /* To prevent -fmax-errors= triggering. */
+ --werrorcount;
+ pp->buffer = &pp_warning_buffer;
+ global_dc->fatal_errors = false;
+ }
+
+ va_start (argp, gmsgid);
+ diagnostic_set_info (&diagnostic, gmsgid, &argp, UNKNOWN_LOCATION,
+ DK_WARNING);
+ diagnostic.option_index = opt;
+ bool ret = report_diagnostic (&diagnostic);
+
+ if (ret && !pp_warning_buffer.flush_p)
+ {
+ warningcount_buffered = 0;
+ werrorcount_buffered = 0;
+ /* Undo the above --werrorcount if not Werror, otherwise werrorcount is
+ correct already. */
+ diagnostic.kind == DK_ERROR
+ ? (++werrorcount_buffered)
+ : (++werrorcount, --warningcount, ++warningcount_buffered);
+
+ pp->buffer = tmp_buffer;
+ global_dc->fatal_errors = fatal_errors;
+ }
+
+ va_end (argp);
+ return ret;
+}
> Well, for nearly every Fortran program, at least for one line multiple
> attempts have to be done by the parser. Thus, if you set a break point in
> gfc_error, you will see "syntax error" messages which never make it to the
> screen. As the test suite checks for excess errors, nearly every test-suite
> file tests this.
Sure, but I would like to test specifically triggering and discarding
the gfc_warning that I converted (or another single one that you
suggest), if this were possible.
> Some of those can probably be simply converted, others either need to
> remain, or one has to setup a proper location (currently, using
> gfc_warning_now would ICE), or one creates a work around and constructs
> manually the error string (at least for the fprintf cases).
Why does it ICE? At worst it should give a wrong location, but the
computation of the offset is fairly similar to what Fortran already
does. Could you give me a patch+testcase that ICEs?
> Taking everything together, I think using libcpp for reading Fortran files
> is a topic for GCC 6. Any suggestion how to properly handle the spacing?
No idea. My knowledge of cpp is limited. I think you need to ask
Dodji, Tom or Joseph (in IRC or a new thread). I was hoping it was
going to be easier to lex Fortran with cpp since cpp can already
preprocess it.
Thanks for the comment. They are very helpful.
Cheers,
Manuel.
