This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)


Hi Keith,

the patch
	42284fdf9d8cdb20c8e833bdbdb2b56977fea525
	http://sourceware.org/ml/gdb-cvs/2010-03/msg00082.html
	dwarf2_physname patchset:
	[RFA] dwarf2_physname FINAL
	http://sourceware.org/ml/gdb-patches/2010-03/msg00220.html

contained three parts:

(a) Drop DW_AT_linkage_name use.
    =>PR 12328 - the const/volatile qualifiers
    =>PR 11734 non-decode_compound - psymtabs no longer having parameter types
 * To be definitely kept in.

(b) switch from classname::any(any) linespec resolving by decode_variable
    to the classname-scope aware decode_compound.
    =>PR 11734 decode_compound - quoting fixup
    =>PR 12273 - decode_compound not aware of minimal symbols
 * Why to apply this part is this mail about.

(c) Some fixes of formerly unsupported linespecs (such as trailing ` const').
 * Partial port to the legacy GDB codebase.

I do not fully understand the reasons for part (b).  The old code is not nice
but IMO neither is the new code (already checked in by the physname patch) due to
the linespec.c caller.  Moreover the new code has shown its regressions.


If the code should be nice I tried archer-jankratochvil-linespec where
linespec is based on the expressions.  Noted by Daniel Jacobowitz before:
	http://sourceware.org/ml/gdb-patches/2009-11/msg00266.html
It still has some regressions but the most common C++ cases work there and
I find it doable with some more time (mostly if the error messages can be
changed).  The are problems with:
 * expression evaluator cares about the function value (=address) where the
   function symbol for linespec is already dropped.
 * linespec should have no side effects.  But the expression evaluator's
   EVAL_AVOID_SIDE_EFFECTS mode cares only about types, not about values.


This "revert" series:
[patch 1/3] is revert of the parts (b)+(c).
[patch 2/3] is application of the 11734 non-decode_compound part psymtabs fix.
[patch 3/3] is reapplication of (c).

Afterwards there remain these regresions:
+FAIL: gdb.cp/pr12273.exp: setting breakpoint at GDB<int>::operator ==
+FAILs in gdb.java/jmisc.exp and gdb.java/jprint.exp
 (IMHO gcj gdb.java/* should not be anything relevant with openjdk now.)

OTOH namespaces work better with these three "revert" patches while it does
not with the patches of yours under review.  With the testfiles

	http://sourceware.org/ml/gdb-patches/2011-01/msg00514.html
../gdb -nx gdb.cp/pr11734 -ex start -ex 'b pr11734::foo()'

Sure the namespaces could be fixed in your patchset, but also `operator =='
could be futher fixed up in these "revert" patches.


That is in general I would be either for futher not-nice fixing up the
pre-physname code or for the expression way like archer-jankratochvil-linespec
does.  Still at the moment your patchset gives the best user experience, just
it is a new untested code which does not make it nice anyway.


Thanks,
Jan


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]