[Bug debug/104025] New: [12 Regression] '-fcompare-debug' failure with enabled warnings

[Bug debug/104025] New: [12 Regression] '-fcompare-debug' failure with enabled warnings

[zsojka at seznam dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

            Bug ID: 104025
           Summary: [12 Regression] '-fcompare-debug' failure with enabled
                    warnings
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: debug
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
                CC: aoliva at gcc dot gnu.org
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu

Created attachment 52187
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52187&action=edit
reduced testcase

Compiler output:
$ x86_64-pc-linux-gnu-gcc -fcompare-debug testcase.C
x86_64-pc-linux-gnu-gcc: error: testcase.C: '-fcompare-debug' failure
$ diff -u *gkd
...
@@ -31,11 +31,11 @@
 (note # 0 0 NOTE_INSN_FUNCTION_BEG)
 (insn # 0 0 2 (set (reg/f:DI 0 ax [86])
         (mem/f/c:DI (plus:DI (reg/f:DI 6 bp)
-                (const_int -8 [0xfffffffffffffff8])) [ this+0 S8 A64]))
"testcase.C":14:16# {*movdi_internal}
+                (const_int -8 [0xfffffffffffffff8])) [ this+0 S8 A64]))
"testcase.C":14:12# {*movdi_internal}
      (nil))
 (insn # 0 0 2 (set (reg:SI 1 dx [orig:82 _1 ] [82])
         (mem:SI (plus:DI (reg/f:DI 0 ax [86])
-                (const_int 4 [0x4])) [ this_6(D)->c.i+0 S4 A32]))
"testcase.C":14:16# {*movsi_internal}
+                (const_int 4 [0x4])) [ this_6(D)->c.i+0 S4 A32]))
"testcase.C":14:12# {*movsi_internal}
      (nil))
 (insn # 0 0 2 (set (reg/f:DI 0 ax [87])
         (mem/f/c:DI (plus:DI (reg/f:DI 6 bp)
$ x86_64-pc-linux-gnu-gcc -fcompare-debug testcase.C -w -c
(no output)


$ x86_64-pc-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r12-6567-20220114130226-gb77e3b4e458-checking-yes-rtl-df-extra-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--with-cloog --with-ppl --with-isl --build=x86_64-pc-linux-gnu
--host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu
--with-ld=/usr/bin/x86_64-pc-linux-gnu-ld
--with-as=/usr/bin/x86_64-pc-linux-gnu-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r12-6567-20220114130226-gb77e3b4e458-checking-yes-rtl-df-extra-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.0.0 20220114 (experimental) (GCC)

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|debug                       |c++
   Target Milestone|---                         |12.0

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note the difference is in the column number 14:16 vs 14:12.

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings since r12-6563-gb8ffa71e4271ae56

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-01-14
     Ever confirmed|0                           |1
                 CC|                            |anthonysharp15 at gmail dot com,
                   |                            |marxin at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
            Summary|[12 Regression]             |[12 Regression]
                   |'-fcompare-debug' failure   |'-fcompare-debug' failure
                   |with enabled warnings       |with enabled warnings since
                   |                            |r12-6563-gb8ffa71e4271ae56

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-6563-gb8ffa71e4271ae56.

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings since r12-6563-gb8ffa71e4271ae56

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Obviously removing "&& warn_missing_template_keyword" will fix the
fcompare-debug but the question is which location is the more correct one?

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings since r12-6563-gb8ffa71e4271ae56

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings since r12-6563-gb8ffa71e4271ae56

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Yeah, the reason that -fcompare-debug fails isn't that -g vs. -g0 would emit
different line number, but that -fcompare-debug uses -w for one of the
compilations so that warnings aren't emitted multiple times and so one
compilation is with -Wmissing-template-keyword and one without.

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings since r12-6563-gb8ffa71e4271ae56

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52213
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52213&action=edit
gcc12-pr104025.patch

Untested fix.  I think the old input_location is the right one.

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings since r12-6563-gb8ffa71e4271ae56

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

--- Comment #6 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> Created attachment 52213 [details]
> gcc12-pr104025.patch
> 
> Untested fix.  I think the old input_location is the right one.

I think the bug is that cp_lexer_rollback_tokens isn't properly updating
input_location.

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings since r12-6563-gb8ffa71e4271ae56

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Maybe.  But how?  Would it push a pair of next_token, input_location ?
I think it isn't easy to recompute input_location after the rollback otherwise.
And, I'm a little bit worried that such a change could be at this point quite
dangerous, while the above patch affects one spot that has changed, we have
lots of rollbacks in various places.

Anyway, patch passed bootstrap/regtest.

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings since r12-6563-gb8ffa71e4271ae56

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:2aa184458a11a7ec8c1b451b8eeff458a24632bb

commit r12-6703-g2aa184458a11a7ec8c1b451b8eeff458a24632bb
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Jan 17 17:16:38 2022 -0500

    c++: input_location and lookahead [PR104025]

    Debug information was getting confused because input_location was different
    depending on whether we had looked ahead to see if the next tokens look
like
    a template argument list.

    I tried resetting input_location in cp_lexer_rollback_tokens itself, but
    that caused regressions, so let's just do it here for now.

            PR c++/104025

    gcc/cp/ChangeLog:

            * parser.cc (saved_token_sentinel::rollback): Call
            cp_lexer_set_source_position.
            (~saved_token_sentinel): Call rollback.

    gcc/testsuite/ChangeLog:

            * g++.dg/warn/pr104025.C: New test.

    Co-authored-by: Jakub Jelinek  <jakub@redhat.com>

[Bug c++/104025] [12 Regression] '-fcompare-debug' failure with enabled warnings since r12-6563-gb8ffa71e4271ae56

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #7)
> Maybe.  But how?  Would it push a pair of next_token, input_location ?
> I think it isn't easy to recompute input_location after the rollback
> otherwise.
> And, I'm a little bit worried that such a change could be at this point
> quite dangerous, while the above patch affects one spot that has changed, we
> have lots of rollbacks in various places.

Fair; doing it in saved_token_sentinel seemed like a reasonable compromise.