diff mbox

[RFA,10/13] Remove unused declaration from value.c

Message ID 20180712205208.32646-11-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey July 12, 2018, 8:52 p.m. UTC 
  This removes an unused declaration from value_fetch_lazy_bitfield, but
leaves the call to check_typedef, because it may be called for effect.

gdb/ChangeLog
2018-07-12  Tom Tromey  <tom@tromey.com>

	* value.c (value_fetch_lazy_bitfield): Call check_typedef for
	effect.
---
 gdb/ChangeLog | 5 +++++
 gdb/value.c   | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Simon Marchi July 13, 2018, 2:52 a.m. UTC | #1 
On 2018-07-12 04:52 PM, Tom Tromey wrote:
> This removes an unused declaration from value_fetch_lazy_bitfield, but
> leaves the call to check_typedef, because it may be called for effect.

Do you know for sure this is necessary (e.g. without this, some test fails),
or you are just being cautious?

Simon
Tom Tromey July 13, 2018, 8:51 p.m. UTC | #2 
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 2018-07-12 04:52 PM, Tom Tromey wrote:
>> This removes an unused declaration from value_fetch_lazy_bitfield, but
>> leaves the call to check_typedef, because it may be called for effect.

Simon> Do you know for sure this is necessary (e.g. without this, some test fails),
Simon> or you are just being cautious?

Just being cautious.  It's entirely possible that this isn't needed.
Maybe removing it is correct in that nothing in the function appears to
need it, and if it causes a bug then that means that some other spot
ought to have called check_typedef.

Tom
Simon Marchi July 13, 2018, 9:49 p.m. UTC | #3 
On 2018-07-13 16:51, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> On 2018-07-12 04:52 PM, Tom Tromey wrote:
>>> This removes an unused declaration from value_fetch_lazy_bitfield, 
>>> but
>>> leaves the call to check_typedef, because it may be called for 
>>> effect.
> 
> Simon> Do you know for sure this is necessary (e.g. without this, some
> test fails),
> Simon> or you are just being cautious?
> 
> Just being cautious.  It's entirely possible that this isn't needed.
> Maybe removing it is correct in that nothing in the function appears to
> need it, and if it causes a bug then that means that some other spot
> ought to have called check_typedef.
> 
> Tom

Then my opinion would be to check if removing it causes any test 
failure.  If not, I'd remove it (in its own commit such as this patch is 
good, so it's easy to bisect if needed).

Simon
Pedro Alves July 16, 2018, 2:01 p.m. UTC | #4 
On 07/13/2018 10:49 PM, Simon Marchi wrote:

> Then my opinion would be to check if removing it causes any test failure.  If not, I'd remove it (in its own commit such as this patch is good, so it's easy to bisect if needed).

Agreed.  If/when we find out it was needed, we can add
a testcase then.

Thanks,
Pedro Alves
Tom Tromey July 16, 2018, 3:34 p.m. UTC | #5 
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Simon> Then my opinion would be to check if removing it causes any test
Simon> failure.  If not, I'd remove it (in its own commit such as this
Simon> patch is good, so it's easy to bisect if needed).

Pedro> Agreed.  If/when we find out it was needed, we can add
Pedro> a testcase then.

I've made this change.

Tom
diff mbox

Patch

diff --git a/gdb/value.c b/gdb/value.c
index 9f9e78ece2b..1747a62980f 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3726,7 +3726,7 @@  value_fetch_lazy_bitfield (struct value *val)
      per bitfield.  It would be even better to read only the containing
      word, but we have no way to record that just specific bits of a
      value have been fetched.  */
-  struct type *type = check_typedef (value_type (val));
+  check_typedef (value_type (val));
   struct value *parent = value_parent (val);
 
   if (value_lazy (parent))