This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Remove some variables in favor of using gdb::optional
On 2019-08-24 7:55 p.m., Simon Marchi wrote:
> On 2019-08-24 07:22, Ruslan Kabatsayev wrote:
>> Since GDB uses C++11, and we don't really rely on conversion to int
>> here, why not use enum class? This would protect from unwanted
>> conversions. Additionally, we'll be forced to use a "scoped" name
>> instead of "prefixed" one, like symbol_nature::unknown vs
>> symbol_nature_unknown, which seems a bit more explicit (will need to
>> do something with "static" though, since it's a keyword).
>
> Good point, I'll try this for the new version.
>
> Simon
This is what I ended up pushing, thanks to both of you for your comments.
>From beadd3e84ed8e652015f07eb4734a6d3b17e79cb Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Sun, 25 Aug 2019 18:09:47 -0400
Subject: [PATCH] dwarf2read: replace gdb::optional<bool> with enum
gdb::optional<bool> is dangerous, because it's easy to do:
if (opt_bool)
when you actually meant
if (*opt_bool)
or vice-versa. The first checks if the optional is set, the second
checks if the wrapped bool is true.
Replace it with an enum that explicitly defines the three possible
states.
gdb/ChangeLog:
* dwarf2read.c (dw2_debug_names_iterator::next): Use enum to
represent whether the symbol is static, dynamic, or we don't
know.
---
gdb/ChangeLog | 6 ++++++
gdb/dwarf2read.c | 15 ++++++++++-----
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cbb83347f1e6..5f64ca6d4a94 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-08-25 Simon Marchi <simon.marchi@efficios.com>
+
+ * dwarf2read.c (dw2_debug_names_iterator::next): Use enum to
+ represent whether the symbol is static, dynamic, or we don't
+ know.
+
2019-08-25 Yoshinori Sato <ysato@users.sourceforge.jp>
* gdb/rx-tdep.c (rx_register_names): New.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index de9755f6ce30..a0b989fd0c2e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -5843,7 +5843,11 @@ dw2_debug_names_iterator::next ()
return NULL;
}
const mapped_debug_names::index_val &indexval = indexval_it->second;
- gdb::optional<bool> is_static;
+ enum class symbol_linkage {
+ unknown,
+ static_,
+ extern_,
+ } symbol_linkage = symbol_linkage::unknown;
dwarf2_per_cu_data *per_cu = NULL;
for (const mapped_debug_names::index_val::attr &attr : indexval.attr_vec)
{
@@ -5895,12 +5899,12 @@ dw2_debug_names_iterator::next ()
case DW_IDX_GNU_internal:
if (!m_map.augmentation_is_gdb)
break;
- is_static = true;
+ symbol_linkage = symbol_linkage::static_;
break;
case DW_IDX_GNU_external:
if (!m_map.augmentation_is_gdb)
break;
- is_static = false;
+ symbol_linkage = symbol_linkage::extern_;
break;
}
}
@@ -5910,10 +5914,11 @@ dw2_debug_names_iterator::next ()
goto again;
/* Check static vs global. */
- if (is_static.has_value () && m_block_index.has_value ())
+ if (symbol_linkage != symbol_linkage::unknown && m_block_index.has_value ())
{
const bool want_static = *m_block_index == STATIC_BLOCK;
- if (want_static != *is_static)
+ const bool symbol_is_static = symbol_linkage == symbol_linkage::static_;
+ if (want_static != symbol_is_static)
goto again;
}
--
2.23.0