Re: [Jim Blandy <jimb@redhat.com>] RFA: Check that `Local' is not in scope when it shouldn't be

[Michael Elizabeth Chastain <mec@shout.net>]

Regarding this patch:

  2002-11-14  Jim Blandy  <jimb@redhat.com>

	* local.exp: Don't expect Local to be in scope in main; it's local
	to foobar.  Check for it there, and check that it's not present in
	main.
	* local.cc (marker2): New function.
	(foobar): Call marker1.
	(main): Call marker2 instead of marker1.

The first part of this patch, "Don't expect Local to be in scope in main;
it's local to foobar.", is approved.  I agree with your analysis and most
of your solution.  I don't like the second call to "runto 'marker2'",
because "runto" restarts gdb needlessly.  I think it's better to do:

  gdb_test "break marker2" ...
  gdb_test "continue" "Break.* marker2 ... at ..."

But the test works correctly as you wrote it, and the existing code 
doesn't, so I approve it and someone can rewrite the second "runto"
after you commit this.

The second part, "... check that it's not present in main",
is not approved.  You wrote the KFAIL like this:

  # setup_kfail "gdb/825"
  gdb_test "ptype Local" "No symbol \"Local\" in current context.*" \
    "Local out of scope (gdb/825)"

There is a coding style issue here.  My vision is that KFAIL's are always
a response to specific known bad output from gdb, like this:

  send_gdb "ptype Local\n"
  gdb_expect {
    -re "No symbol \"Local in current context.\r\n$gdb_prompt $" {
      pass "ptype Local"
    }
    -re "... big regular expression that matches the correct declaration of Local from the wrong scope ..." {
      # comment here about which configurations we expect this for
      kfail "gdb/825" "ptype Local"
    }
    -re ".*$gdb_prompt $" {
      fail "ptype Local"
    }
    timeout {
      fail "ptype Local (timeout)"
    }
  }

This way, the test can still generate FAIL if gdb starts acting in a
new unknown way.  The first test actually does this; it went from FAIL
(gdb/483) to an unmarked FAIL, which indicated that there was a problem
(which turned out to be in the test script itself).

I think this issue needs some discussion and then a decision from
FernandoN or AndrewC so I'd like to hold off on the second test in
this patch while we resolve this.

(I spent too much time on this patch because I started doing my own
update of Local.exp with my style of KFAIL's everywhere.  Then I decided
it's better just to respond to JimB and work on an all-updated local.exp
later.)

Michael C