Re: [PATCH 2/2] replace several uses of the anon namespace with GCC_FINAL

[David Malcolm <dmalcolm at redhat dot com>]

On Thu, 2015-08-13 at 01:58 +0200, Markus Trippelsdorf wrote:
> On 2015.08.12 at 13:03 -0600, Jeff Law wrote:
> > On 08/12/2015 12:57 PM, Richard Sandiford wrote:
> > > Jeff Law <law@redhat.com> writes:
> > >> On 08/10/2015 06:05 AM, tbsaunde+gcc@tbsaunde.org wrote:
> > >>> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> > >>>
> > >>> Hi,
> > >>>
> > >>> In many places gcc puts classes in the anon namespace so the compiler can tell
> > >>> they do not get inheritted from to enable better devirtualization.  However
> > >>> debugging code in the anon namespace can be a pain, and the same thing can be
> > >>> accomplished more directly by marking the classes as final.  When
> > >>> bootstrapping
> > >>> stage 3 should always be built in C++14 mode now, and of course will always be
> > >>> newer than gcc 4.7, so these classes will always be marked as final there.
> > >>> AIUI cross compilers are supposed to be built with recent gcc, which I would
> > >>> tend to think implies newer than 4.7, so they should also be built with these
> > >>> classes marked as final.  I believe that means in all important cases
> > >>> this works just as well as the anon namespace.
> > >>>
> > >>> bootstrapped + regtested on x86_64-linux-gnu, ok?
> > >>>
> > >>> Trev
> > >>>
> > >>>
> > >>> gcc/ChangeLog:
> > >>>
> > >>> 2015-08-10  Trevor Saunders  <tbsaunde@tbsaunde.org>
> > >>>
> > >>> 	* compare-elim.c, dce.c, dse.c, gimple-ssa-isolate-paths.c,
> > >>> 	gimple-ssa-strength-reduction.c, graphite.c, init-regs.c,
> > >>> 	ipa-pure-const.c, ipa-visibility.c, ipa.c, mode-switching.c,
> > >>> 	omp-low.c, reorg.c, sanopt.c, trans-mem.c, tree-eh.c,
> > >>> 	tree-if-conv.c, tree-ssa-copyrename.c, tree-ssa-dce.c,
> > >>> 	tree-ssa-dom.c, tree-ssa-dse.c, tree-ssa-forwprop.c,
> > >>> 	tree-ssa-sink.c, tree-ssanames.c, tree-stdarg.c, tree-tailcall.c,
> > >>> 	tree-vect-generic.c, tree.c, ubsan.c, var-tracking.c,
> > >>> 	vtable-verify.c, web.c: Use GCC_FINAL instead of the anonymous
> > >>> namespace.
> > >> OK.
> > >
> > > I was hoping someone else was going to speak up since I seem
> > > to have been posting a few negative messages recently, but I think
> > > this is really a step in the wrong direction.  I think the code
> > > was using anonymous namespaces in exactly the way they were
> > > intended to be used.
> > No need to worry about seeming to be negative.
> > 
> > 
> > The problem is you can't get to stuff in the anonymous namespace easily 
> > in the debugger.  There was talk of fixing that, but I don't think it 
> > ever went forward on the gdb side.
> > 
> > If gdb were to get fixed so that debugging this stuff was easier, then 
> > I'd fully support putting things back into the anonymous namespace.
> 
> For the record here the gdb bug in question:
> https://sourceware.org/bugzilla/show_bug.cgi?id=16874
> It even has a patch attached, that improves the issue.

Sadly that seems to have stalled.

FWIW, our gcc/gdbhooks.py adds in a "break-on-pass" gdb command to work
around this:
https://gcc.gnu.org/ml/gcc-patches/2014-07/msg02011.html

  (gdb) break-on-pass pass_final
  Breakpoint 6 at 0x8396ba: file ../../src/gcc/final.c, line 4526.
  (gdb) cont
  Continuing.
  Breakpoint 6, (anonymous namespace)::pass_final::execute (this=0x17fb990) at ../../src/gcc/final.c:4526
  4526	  virtual unsigned int execute (function *) { return rest_of_handle_final (); }
  (gdb)

Though clearly hacking around it this way is suboptimal.

Dave