* [PATCH 6/7] gdb_xml_parser: make data fields private and make more functions methods
2017-04-10 12:22 [PATCH 0/7] C++fy xml-support.c and more gdb::optional Pedro Alves
@ 2017-04-10 12:22 ` Pedro Alves
2017-04-10 12:22 ` [PATCH 2/7] xml-support.c: Use std::vector Pedro Alves
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2017-04-10 12:22 UTC (permalink / raw)
To: gdb-patches
This patch makes the data fields of gdb_xml_parser private, and makes
more functions be gdb_xml_parser methods. This is mostly for better
encapsulation.
Some free functions have their parsing-related guts converted to
methods, while the free functions remain, as they're used as expat
callbacks. Now their only job is to be small shims that restore back
the gdb_xml_parser type, defer work to the corresponding method, and
make sure C++ exceptions don't cross expat.
More C++-fycation of the XML parsers built on top of gdb_xml_parser
could follow, but this was my stopping point.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* xml-support.c (gdb_xml_parser) <use_dtd, dtd_name, parse,
vdebug, verror, body_text, start_element, end_element, name,
user_data, set_is_xinclude, set_error, expat_parser>: New methods.
<name, user_data, expat_parser, scopes, error, last_line, dtd_name,
is_xinclude>: Make private and add m_ prefix.
(gdb_xml_parser::body_text): New method, based on ...
(gdb_xml_body_text): ... this. Adjust.
(gdb_xml_parser::vdebug): New method, based on ...
(gdb_xml_debug): ... this. Adjust.
(gdb_xml_parser::verror): New method, based on ...
(gdb_xml_error): ... this. Adjust.
(gdb_xml_parser::start_element): New method, based on ...
(gdb_xml_start_element): ... this. Adjust.
(gdb_xml_start_element_wrapper): Defer to
gdb_xml_parser::start_element and gdb_xml_parser::set_error.
(gdb_xml_parser::end_element): New method, based on ...
(gdb_xml_end_element_wrapper): ... this. Adjust.
(gdb_xml_parser::~gdb_xml_parser): Adjust.
(gdb_xml_parser::gdb_xml_parser): Adjust to field renames.
(gdb_xml_parser::use_dtd): New method, based on ...
(gdb_xml_use_dtd): ... this. Adjust.
(gdb_xml_parser::parse): New method, based on ...
(gdb_xml_parse): ... this. Adjust.
(gdb_xml_parse_quick): Adjust to call the parser's parse method.
(xinclude_start_include): Adjust to call the parser's name method.
(xml_xinclude_default, xml_xinclude_start_doctype)
(xml_xinclude_end_doctype): Adjust to call the parser's user_data
method.
(xml_process_xincludes): Adjust to call parser methods.
* xml-support.h (gdb_xml_use_dtd, gdb_xml_parse): Delete
declarations.
---
gdb/xml-support.c | 341 +++++++++++++++++++++++++++++++++---------------------
gdb/xml-support.h | 14 ---
2 files changed, 206 insertions(+), 149 deletions(-)
diff --git a/gdb/xml-support.c b/gdb/xml-support.c
index 91e31d9..9595246 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -72,73 +72,156 @@ struct gdb_xml_parser
void *user_data);
~gdb_xml_parser();
- XML_Parser expat_parser; /* The underlying expat parser. */
+ /* Associate DTD_NAME, which must be the name of a compiled-in DTD,
+ with the parser. */
+ void use_dtd (const char *dtd_name);
+
+ /* Return the name of the expected / default DTD, if specified. */
+ const char *dtd_name ()
+ { return m_dtd_name; }
+
+ /* Invoke the parser on BUFFER. BUFFER is the data to parse, which
+ should be NUL-terminated.
+
+ The return value is 0 for success or -1 for error. It may throw,
+ but only if something unexpected goes wrong during parsing; parse
+ errors will be caught, warned about, and reported as failure. */
+ int parse (const char *buffer);
+
+ /* Issue a debugging message. */
+ void vdebug (const char *format, va_list ap)
+ ATTRIBUTE_PRINTF (2, 0);
+
+ /* Issue an error message, and stop parsing. */
+ void verror (const char *format, va_list ap)
+ ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 0);
+
+ void body_text (const XML_Char *text, int length);
+ void start_element (const XML_Char *name, const XML_Char **attrs);
+ void end_element (const XML_Char *name);
+
+ /* Return the name of this parser. */
+ const char *name ()
+ { return m_name; }
+
+ /* Return the user's callback data, for handlers. */
+ void *user_data ()
+ { return m_user_data; };
+
+ /* Are we the special <xi:include> parser? */
+ void set_is_xinclude (bool is_xinclude)
+ { m_is_xinclude = is_xinclude; }
+
+ /* A thrown error, if any. */
+ void set_error (gdb_exception error)
+ {
+ m_error = error;
+#ifdef HAVE_XML_STOPPARSER
+ XML_StopParser (m_expat_parser, XML_FALSE);
+#endif
+ }
+
+ /* Return the underlying expat parser. */
+ XML_Parser expat_parser ()
+ { return m_expat_parser; }
+
+private:
+ /* The underlying expat parser. */
+ XML_Parser m_expat_parser;
+
+ /* Name of this parser. */
+ const char *m_name;
- const char *name; /* Name of this parser. */
- void *user_data; /* The user's callback data, for handlers. */
+ /* The user's callback data, for handlers. */
+ void *m_user_data;
/* Scoping stack. */
- std::vector<scope_level> scopes;
+ std::vector<scope_level> m_scopes;
- struct gdb_exception error; /* A thrown error, if any. */
- int last_line; /* The line of the thrown error, or 0. */
+/* A thrown error, if any. */
+ struct gdb_exception m_error;
- const char *dtd_name; /* The name of the expected / default DTD,
- if specified. */
- bool is_xinclude; /* Are we the special <xi:include> parser? */
+ /* The line of the thrown error, or 0. */
+ int m_last_line;
+
+ /* The name of the expected / default DTD, if specified. */
+ const char *m_dtd_name;
+
+ /* Are we the special <xi:include> parser? */
+ bool m_is_xinclude;
};
/* Process some body text. We accumulate the text for later use; it's
wrong to do anything with it immediately, because a single block of
text might be broken up into multiple calls to this function. */
+void
+gdb_xml_parser::body_text (const XML_Char *text, int length)
+{
+ if (m_error.reason < 0)
+ return;
+
+ scope_level &scope = m_scopes.back ();
+ scope.body.append (text, length);
+}
+
static void
gdb_xml_body_text (void *data, const XML_Char *text, int length)
{
struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data;
- if (parser->error.reason < 0)
- return;
-
- scope_level &scope = parser->scopes.back ();
- scope.body.append (text, length);
+ parser->body_text (text, length);
}
/* Issue a debugging message from one of PARSER's handlers. */
void
-gdb_xml_debug (struct gdb_xml_parser *parser, const char *format, ...)
+gdb_xml_parser::vdebug (const char *format, va_list ap)
{
- int line = XML_GetCurrentLineNumber (parser->expat_parser);
- va_list ap;
+ int line = XML_GetCurrentLineNumber (m_expat_parser);
char *message;
- if (!debug_xml)
- return;
-
- va_start (ap, format);
message = xstrvprintf (format, ap);
if (line)
fprintf_unfiltered (gdb_stderr, "%s (line %d): %s\n",
- parser->name, line, message);
+ m_name, line, message);
else
fprintf_unfiltered (gdb_stderr, "%s: %s\n",
- parser->name, message);
+ m_name, message);
xfree (message);
}
+void
+gdb_xml_debug (struct gdb_xml_parser *parser, const char *format, ...)
+{
+ if (!debug_xml)
+ return;
+
+ va_list ap;
+ va_start (ap, format);
+ parser->vdebug (format, ap);
+ va_end (ap);
+}
+
/* Issue an error message from one of PARSER's handlers, and stop
parsing. */
void
+gdb_xml_parser::verror (const char *format, va_list ap)
+{
+ int line = XML_GetCurrentLineNumber (m_expat_parser);
+
+ m_last_line = line;
+ throw_verror (XML_PARSE_ERROR, format, ap);
+}
+
+void
gdb_xml_error (struct gdb_xml_parser *parser, const char *format, ...)
{
- int line = XML_GetCurrentLineNumber (parser->expat_parser);
va_list ap;
-
- parser->last_line = line;
va_start (ap, format);
- throw_verror (XML_PARSE_ERROR, format, ap);
+ parser->verror (format, ap);
+ va_end (ap);
}
/* Find the attribute named NAME in the set of parsed attributes
@@ -171,15 +254,16 @@ gdb_xml_values_cleanup (void *data)
VEC_free (gdb_xml_value_s, *values);
}
-/* Handle the start of an element. DATA is our local XML parser, NAME
- is the element, and ATTRS are the names and values of this
- element's attributes. */
+/* Handle the start of an element. NAME is the element, and ATTRS are
+ the names and values of this element's attributes. */
-static void
-gdb_xml_start_element (void *data, const XML_Char *name,
- const XML_Char **attrs)
+void
+gdb_xml_parser::start_element (const XML_Char *name,
+ const XML_Char **attrs)
{
- struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data;
+ if (m_error.reason < 0)
+ return;
+
const struct gdb_xml_element *element;
const struct gdb_xml_attribute *attribute;
VEC(gdb_xml_value_s) *attributes = NULL;
@@ -194,12 +278,12 @@ gdb_xml_start_element (void *data, const XML_Char *name,
recursion unrolls all such elements will have been popped back
already, but if one of those pushes reallocates the vector,
previous element references will be invalidated. */
- parser->scopes.emplace_back ();
+ m_scopes.emplace_back ();
/* Get a reference to the current scope. */
- scope_level &scope = parser->scopes[parser->scopes.size () - 2];
+ scope_level &scope = m_scopes[m_scopes.size () - 2];
- gdb_xml_debug (parser, _("Entering element <%s>"), name);
+ gdb_xml_debug (this, _("Entering element <%s>"), name);
/* Find this element in the list of the current scope's allowed
children. Record that we've seen it. */
@@ -215,21 +299,21 @@ gdb_xml_start_element (void *data, const XML_Char *name,
/* If we're working on XInclude, <xi:include> can be the child
of absolutely anything. Copy the previous scope's element
list into the new scope even if there was no match. */
- if (parser->is_xinclude)
+ if (m_is_xinclude)
{
- XML_DefaultCurrent (parser->expat_parser);
+ XML_DefaultCurrent (m_expat_parser);
- scope_level &unknown_scope = parser->scopes.back ();
+ scope_level &unknown_scope = m_scopes.back ();
unknown_scope.elements = scope.elements;
return;
}
- gdb_xml_debug (parser, _("Element <%s> unknown"), name);
+ gdb_xml_debug (this, _("Element <%s> unknown"), name);
return;
}
if (!(element->flags & GDB_XML_EF_REPEATABLE) && (seen & scope.seen))
- gdb_xml_error (parser, _("Element <%s> only expected once"), name);
+ gdb_xml_error (this, _("Element <%s> only expected once"), name);
scope.seen |= seen;
@@ -253,14 +337,14 @@ gdb_xml_start_element (void *data, const XML_Char *name,
if (*p != NULL && val == NULL)
{
- gdb_xml_debug (parser, _("Attribute \"%s\" missing a value"),
+ gdb_xml_debug (this, _("Attribute \"%s\" missing a value"),
attribute->name);
continue;
}
if (*p == NULL && !(attribute->flags & GDB_XML_AF_OPTIONAL))
{
- gdb_xml_error (parser, _("Required attribute \"%s\" of "
+ gdb_xml_error (this, _("Required attribute \"%s\" of "
"<%s> not specified"),
attribute->name, element->name);
continue;
@@ -269,11 +353,11 @@ gdb_xml_start_element (void *data, const XML_Char *name,
if (*p == NULL)
continue;
- gdb_xml_debug (parser, _("Parsing attribute %s=\"%s\""),
+ gdb_xml_debug (this, _("Parsing attribute %s=\"%s\""),
attribute->name, val);
if (attribute->handler)
- parsed_value = attribute->handler (parser, attribute, val);
+ parsed_value = attribute->handler (this, attribute, val);
else
parsed_value = xstrdup (val);
@@ -296,19 +380,19 @@ gdb_xml_start_element (void *data, const XML_Char *name,
break;
if (attribute == NULL || attribute->name == NULL)
- gdb_xml_debug (parser, _("Ignoring unknown attribute %s"), *p);
+ gdb_xml_debug (this, _("Ignoring unknown attribute %s"), *p);
}
}
/* Call the element handler if there is one. */
if (element->start_handler)
- element->start_handler (parser, element, parser->user_data, attributes);
+ element->start_handler (this, element, m_user_data, attributes);
/* Fill in a new scope level. Note that we must delay getting a
back reference till here because above we might have recursed,
which may have reallocated the vector which invalidates
iterators/pointers/references. */
- scope_level &new_scope = parser->scopes.back ();
+ scope_level &new_scope = m_scopes.back ();
new_scope.element = element;
new_scope.elements = element->children;
@@ -324,41 +408,37 @@ gdb_xml_start_element_wrapper (void *data, const XML_Char *name,
{
struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data;
- if (parser->error.reason < 0)
- return;
-
TRY
{
- gdb_xml_start_element (data, name, attrs);
+ parser->start_element (name, attrs);
}
CATCH (ex, RETURN_MASK_ALL)
{
- parser->error = ex;
-#ifdef HAVE_XML_STOPPARSER
- XML_StopParser (parser->expat_parser, XML_FALSE);
-#endif
+ parser->set_error (ex);
}
END_CATCH
}
-/* Handle the end of an element. PARSER is our local XML parser, and
- NAME is the current element. */
+/* Handle the end of an element. NAME is the current element. */
-static void
-gdb_xml_end_element (gdb_xml_parser *parser, const XML_Char *name)
+void
+gdb_xml_parser::end_element (const XML_Char *name)
{
- struct scope_level *scope = &parser->scopes.back ();
+ if (m_error.reason < 0)
+ return;
+
+ struct scope_level *scope = &m_scopes.back ();
const struct gdb_xml_element *element;
unsigned int seen;
- gdb_xml_debug (parser, _("Leaving element <%s>"), name);
+ gdb_xml_debug (this, _("Leaving element <%s>"), name);
for (element = scope->elements, seen = 1;
element != NULL && element->name != NULL;
element++, seen <<= 1)
if ((scope->seen & seen) == 0
&& (element->flags & GDB_XML_EF_OPTIONAL) == 0)
- gdb_xml_error (parser, _("Required element <%s> is missing"),
+ gdb_xml_error (this, _("Required element <%s> is missing"),
element->name);
/* Call the element processor. */
@@ -383,14 +463,14 @@ gdb_xml_end_element (gdb_xml_parser *parser, const XML_Char *name)
body++;
}
- scope->element->end_handler (parser, scope->element, parser->user_data,
- body);
+ scope->element->end_handler (this, scope->element,
+ m_user_data, body);
}
else if (scope->element == NULL)
- XML_DefaultCurrent (parser->expat_parser);
+ XML_DefaultCurrent (m_expat_parser);
/* Pop the scope level. */
- parser->scopes.pop_back ();
+ m_scopes.pop_back ();
}
/* Wrapper for gdb_xml_end_element, to prevent throwing exceptions
@@ -401,19 +481,13 @@ gdb_xml_end_element_wrapper (void *data, const XML_Char *name)
{
struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data;
- if (parser->error.reason < 0)
- return;
-
TRY
{
- gdb_xml_end_element (parser, name);
+ parser->end_element (name);
}
CATCH (ex, RETURN_MASK_ALL)
{
- parser->error = ex;
-#ifdef HAVE_XML_STOPPARSER
- XML_StopParser (parser->expat_parser, XML_FALSE);
-#endif
+ parser->set_error (ex);
}
END_CATCH
}
@@ -422,34 +496,34 @@ gdb_xml_end_element_wrapper (void *data, const XML_Char *name)
gdb_xml_parser::~gdb_xml_parser ()
{
- XML_ParserFree (this->expat_parser);
+ XML_ParserFree (m_expat_parser);
}
/* Initialize a parser. */
-gdb_xml_parser::gdb_xml_parser (const char *name_,
+gdb_xml_parser::gdb_xml_parser (const char *name,
const gdb_xml_element *elements,
- void *user_data_)
- : name (name_),
- user_data (user_data_),
- error (exception_none),
- last_line (0),
- dtd_name (NULL),
- is_xinclude (false)
+ void *user_data)
+ : m_name (name),
+ m_user_data (user_data),
+ m_error (exception_none),
+ m_last_line (0),
+ m_dtd_name (NULL),
+ m_is_xinclude (false)
{
- this->expat_parser = XML_ParserCreateNS (NULL, '!');
- if (this->expat_parser == NULL)
+ m_expat_parser = XML_ParserCreateNS (NULL, '!');
+ if (m_expat_parser == NULL)
malloc_failure (0);
- XML_SetUserData (this->expat_parser, this);
+ XML_SetUserData (m_expat_parser, this);
/* Set the callbacks. */
- XML_SetElementHandler (this->expat_parser, gdb_xml_start_element_wrapper,
+ XML_SetElementHandler (m_expat_parser, gdb_xml_start_element_wrapper,
gdb_xml_end_element_wrapper);
- XML_SetCharacterDataHandler (this->expat_parser, gdb_xml_body_text);
+ XML_SetCharacterDataHandler (m_expat_parser, gdb_xml_body_text);
/* Initialize the outer scope. */
- this->scopes.emplace_back (elements);
+ m_scopes.emplace_back (elements);
}
/* External entity handler. The only external entities we support
@@ -463,19 +537,20 @@ gdb_xml_fetch_external_entity (XML_Parser expat_parser,
const XML_Char *systemId,
const XML_Char *publicId)
{
- struct gdb_xml_parser *parser
- = (struct gdb_xml_parser *) XML_GetUserData (expat_parser);
XML_Parser entity_parser;
const char *text;
enum XML_Status status;
if (systemId == NULL)
{
- text = fetch_xml_builtin (parser->dtd_name);
+ gdb_xml_parser *parser
+ = (gdb_xml_parser *) XML_GetUserData (expat_parser);
+
+ text = fetch_xml_builtin (parser->dtd_name ());
if (text == NULL)
internal_error (__FILE__, __LINE__,
_("could not locate built-in DTD %s"),
- parser->dtd_name);
+ parser->dtd_name ());
}
else
{
@@ -484,7 +559,8 @@ gdb_xml_fetch_external_entity (XML_Parser expat_parser,
return XML_STATUS_ERROR;
}
- entity_parser = XML_ExternalEntityParserCreate (expat_parser, context, NULL);
+ entity_parser = XML_ExternalEntityParserCreate (expat_parser,
+ context, NULL);
/* Don't use our handlers for the contents of the DTD. Just let expat
process it. */
@@ -500,23 +576,20 @@ gdb_xml_fetch_external_entity (XML_Parser expat_parser,
return status;
}
-/* Associate DTD_NAME, which must be the name of a compiled-in DTD,
- with PARSER. */
-
void
-gdb_xml_use_dtd (struct gdb_xml_parser *parser, const char *dtd_name)
+gdb_xml_parser::use_dtd (const char *dtd_name)
{
enum XML_Error err;
- parser->dtd_name = dtd_name;
+ m_dtd_name = dtd_name;
- XML_SetParamEntityParsing (parser->expat_parser,
+ XML_SetParamEntityParsing (m_expat_parser,
XML_PARAM_ENTITY_PARSING_UNLESS_STANDALONE);
- XML_SetExternalEntityRefHandler (parser->expat_parser,
+ XML_SetExternalEntityRefHandler (m_expat_parser,
gdb_xml_fetch_external_entity);
/* Even if no DTD is provided, use the built-in DTD anyway. */
- err = XML_UseForeignDTD (parser->expat_parser, XML_TRUE);
+ err = XML_UseForeignDTD (m_expat_parser, XML_TRUE);
if (err != XML_ERROR_NONE)
internal_error (__FILE__, __LINE__,
_("XML_UseForeignDTD failed: %s"),
@@ -531,41 +604,41 @@ gdb_xml_use_dtd (struct gdb_xml_parser *parser, const char *dtd_name)
errors will be caught, warned about, and reported as failure. */
int
-gdb_xml_parse (struct gdb_xml_parser *parser, const char *buffer)
+gdb_xml_parser::parse (const char *buffer)
{
enum XML_Status status;
const char *error_string;
- gdb_xml_debug (parser, _("Starting:\n%s"), buffer);
+ gdb_xml_debug (this, _("Starting:\n%s"), buffer);
- status = XML_Parse (parser->expat_parser, buffer, strlen (buffer), 1);
+ status = XML_Parse (m_expat_parser, buffer, strlen (buffer), 1);
- if (status == XML_STATUS_OK && parser->error.reason == 0)
+ if (status == XML_STATUS_OK && m_error.reason == 0)
return 0;
- if (parser->error.reason == RETURN_ERROR
- && parser->error.error == XML_PARSE_ERROR)
+ if (m_error.reason == RETURN_ERROR
+ && m_error.error == XML_PARSE_ERROR)
{
- gdb_assert (parser->error.message != NULL);
- error_string = parser->error.message;
+ gdb_assert (m_error.message != NULL);
+ error_string = m_error.message;
}
else if (status == XML_STATUS_ERROR)
{
- enum XML_Error err = XML_GetErrorCode (parser->expat_parser);
+ enum XML_Error err = XML_GetErrorCode (m_expat_parser);
error_string = XML_ErrorString (err);
}
else
{
- gdb_assert (parser->error.reason < 0);
- throw_exception (parser->error);
+ gdb_assert (m_error.reason < 0);
+ throw_exception (m_error);
}
- if (parser->last_line != 0)
- warning (_("while parsing %s (at line %d): %s"), parser->name,
- parser->last_line, error_string);
+ if (m_last_line != 0)
+ warning (_("while parsing %s (at line %d): %s"), m_name,
+ m_last_line, error_string);
else
- warning (_("while parsing %s: %s"), parser->name, error_string);
+ warning (_("while parsing %s: %s"), m_name, error_string);
return -1;
}
@@ -577,8 +650,8 @@ gdb_xml_parse_quick (const char *name, const char *dtd_name,
{
gdb_xml_parser parser (name, elements, user_data);
if (dtd_name != NULL)
- gdb_xml_use_dtd (&parser, dtd_name);
- return gdb_xml_parse (&parser, document);
+ parser.use_dtd (dtd_name);
+ return parser.parse (document);
}
/* Parse a field VALSTR that we expect to contain an integer value.
@@ -749,7 +822,8 @@ xinclude_start_include (struct gdb_xml_parser *parser,
gdb_xml_error (parser, _("Could not load XML document \"%s\""), href);
back_to = make_cleanup (xfree, text);
- if (!xml_process_xincludes (data->output, parser->name, text, data->fetcher,
+ if (!xml_process_xincludes (data->output, parser->name (),
+ text, data->fetcher,
data->fetcher_baton,
data->include_depth + 1))
gdb_xml_error (parser, _("Parsing \"%s\" failed"), href);
@@ -774,8 +848,7 @@ static void XMLCALL
xml_xinclude_default (void *data_, const XML_Char *s, int len)
{
struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data_;
- struct xinclude_parsing_data *data
- = (struct xinclude_parsing_data *) parser->user_data;
+ xinclude_parsing_data *data = (xinclude_parsing_data *) parser->user_data ();
/* If we are inside of e.g. xi:include or the DTD, don't save this
string. */
@@ -793,8 +866,7 @@ xml_xinclude_start_doctype (void *data_, const XML_Char *doctypeName,
int has_internal_subset)
{
struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data_;
- struct xinclude_parsing_data *data
- = (struct xinclude_parsing_data *) parser->user_data;
+ xinclude_parsing_data *data = (xinclude_parsing_data *) parser->user_data ();
/* Don't print out the doctype, or the contents of the DTD internal
subset, if any. */
@@ -805,8 +877,7 @@ static void XMLCALL
xml_xinclude_end_doctype (void *data_)
{
struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data_;
- struct xinclude_parsing_data *data
- = (struct xinclude_parsing_data *) parser->user_data;
+ xinclude_parsing_data *data = (xinclude_parsing_data *) parser->user_data ();
data->skip_depth--;
}
@@ -843,25 +914,25 @@ xml_process_xincludes (std::string &result,
xinclude_parsing_data data (result, fetcher, fetcher_baton, depth);
gdb_xml_parser parser (name, xinclude_elements, &data);
- parser.is_xinclude = true;
+ parser.set_is_xinclude (true);
- XML_SetCharacterDataHandler (parser.expat_parser, NULL);
- XML_SetDefaultHandler (parser.expat_parser, xml_xinclude_default);
+ XML_SetCharacterDataHandler (parser.expat_parser (), NULL);
+ XML_SetDefaultHandler (parser.expat_parser (), xml_xinclude_default);
/* Always discard the XML version declarations; the only important
thing this provides is encoding, and our result will have been
converted to UTF-8. */
- XML_SetXmlDeclHandler (parser.expat_parser, xml_xinclude_xml_decl);
+ XML_SetXmlDeclHandler (parser.expat_parser (), xml_xinclude_xml_decl);
if (depth > 0)
/* Discard the doctype for included documents. */
- XML_SetDoctypeDeclHandler (parser.expat_parser,
+ XML_SetDoctypeDeclHandler (parser.expat_parser (),
xml_xinclude_start_doctype,
xml_xinclude_end_doctype);
- gdb_xml_use_dtd (&parser, "xinclude.dtd");
+ parser.use_dtd ("xinclude.dtd");
- if (gdb_xml_parse (&parser, text) == 0)
+ if (parser.parse (text) == 0)
{
if (depth == 0)
gdb_xml_debug (&parser, _("XInclude processing succeeded."));
diff --git a/gdb/xml-support.h b/gdb/xml-support.h
index 8248a32..f9ea64d 100644
--- a/gdb/xml-support.h
+++ b/gdb/xml-support.h
@@ -172,20 +172,6 @@ struct gdb_xml_element
gdb_xml_element_end_handler *end_handler;
};
-/* Associate DTD_NAME, which must be the name of a compiled-in DTD,
- with PARSER. */
-
-void gdb_xml_use_dtd (struct gdb_xml_parser *parser, const char *dtd_name);
-
-/* Invoke PARSER on BUFFER. BUFFER is the data to parse, which
- should be NUL-terminated.
-
- The return value is 0 for success or -1 for error. It may throw,
- but only if something unexpected goes wrong during parsing; parse
- errors will be caught, warned about, and reported as failure. */
-
-int gdb_xml_parse (struct gdb_xml_parser *parser, const char *buffer);
-
/* Parse a XML document. DOCUMENT is the data to parse, which should
be NUL-terminated. If non-NULL, use the compiled-in DTD named
DTD_NAME to drive the parsing.
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/7] xml-support.c: Use std::vector
2017-04-10 12:22 [PATCH 0/7] C++fy xml-support.c and more gdb::optional Pedro Alves
2017-04-10 12:22 ` [PATCH 6/7] gdb_xml_parser: make data fields private and make more functions methods Pedro Alves
@ 2017-04-10 12:22 ` Pedro Alves
2017-04-10 12:22 ` [PATCH 5/7] xml-support.c: Use std::string for growing string buffer Pedro Alves
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2017-04-10 12:22 UTC (permalink / raw)
To: gdb-patches
scope_level::scope_level needed both a move ctor and a dtor explicitly
coded, but those will be eliminated in a following patch.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* xml-support.c: Include <vector>.
(scope_level::scope_level(const gdb_xml_element *))
(scope_level::scope_level(scope_level&&)): New.
(scope_level::~scope_level): New.
(scope_level_s): Delete.
(gdb_xml_parser::scopes): Now a std::vector.
(gdb_xml_body_text, gdb_xml_start_element, gdb_xml_end_element):
Use std::vector.
(gdb_xml_parser::~gdb_xml_parser): Remove now unnecessary
scope cleanup code.
(gdb_xml_parser::gdb_xml_parser): Remove explicit initialization
of the scopes member. Use std::vector::emplace_back.
---
gdb/xml-support.c | 112 +++++++++++++++++++++++++++++-------------------------
1 file changed, 61 insertions(+), 51 deletions(-)
diff --git a/gdb/xml-support.c b/gdb/xml-support.c
index d9dc735..b19f81a 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -22,6 +22,7 @@
#include "xml-support.h"
#include "filestuff.h"
#include "safe-ctype.h"
+#include <vector>
/* Debugging flag. */
static int debug_xml;
@@ -42,6 +43,32 @@ static int debug_xml;
nesting. */
struct scope_level
{
+ explicit scope_level (const gdb_xml_element *elements_ = NULL)
+ : elements (elements_),
+ element (NULL),
+ seen (0),
+ body (NULL)
+ {}
+
+ scope_level (scope_level &&other) noexcept
+ : elements (other.elements),
+ element (other.element),
+ seen (other.seen),
+ body (other.body)
+ {
+ if (this != &other)
+ other.body = NULL;
+ }
+
+ ~scope_level ()
+ {
+ if (this->body)
+ {
+ obstack_free (this->body, NULL);
+ xfree (this->body);
+ }
+ }
+
/* Elements we allow at this level. */
const struct gdb_xml_element *elements;
@@ -52,11 +79,9 @@ struct scope_level
optional and repeatable checking). */
unsigned int seen;
- /* Body text accumulation. */
+ /* Body text accumulation. This is an owning pointer. */
struct obstack *body;
};
-typedef struct scope_level scope_level_s;
-DEF_VEC_O(scope_level_s);
/* The parser itself, and our additional state. */
struct gdb_xml_parser
@@ -71,7 +96,8 @@ struct gdb_xml_parser
const char *name; /* Name of this parser. */
void *user_data; /* The user's callback data, for handlers. */
- VEC(scope_level_s) *scopes; /* Scoping stack. */
+ /* Scoping stack. */
+ std::vector<scope_level> scopes;
struct gdb_exception error; /* A thrown error, if any. */
int last_line; /* The line of the thrown error, or 0. */
@@ -89,18 +115,19 @@ static void
gdb_xml_body_text (void *data, const XML_Char *text, int length)
{
struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data;
- struct scope_level *scope = VEC_last (scope_level_s, parser->scopes);
if (parser->error.reason < 0)
return;
- if (scope->body == NULL)
+ scope_level &scope = parser->scopes.back ();
+
+ if (scope.body == NULL)
{
- scope->body = XCNEW (struct obstack);
- obstack_init (scope->body);
+ scope.body = XCNEW (struct obstack);
+ obstack_init (scope.body);
}
- obstack_grow (scope->body, text, length);
+ obstack_grow (scope.body, text, length);
}
/* Issue a debugging message from one of PARSER's handlers. */
@@ -179,8 +206,6 @@ gdb_xml_start_element (void *data, const XML_Char *name,
const XML_Char **attrs)
{
struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data;
- struct scope_level *scope;
- struct scope_level new_scope;
const struct gdb_xml_element *element;
const struct gdb_xml_attribute *attribute;
VEC(gdb_xml_value_s) *attributes = NULL;
@@ -189,11 +214,16 @@ gdb_xml_start_element (void *data, const XML_Char *name,
/* Push an error scope. If we return or throw an exception before
filling this in, it will tell us to ignore children of this
- element. */
- VEC_reserve (scope_level_s, parser->scopes, 1);
- scope = VEC_last (scope_level_s, parser->scopes);
- memset (&new_scope, 0, sizeof (new_scope));
- VEC_quick_push (scope_level_s, parser->scopes, &new_scope);
+ element. Note we don't take a reference to the element yet
+ because further below we'll process the element which may recurse
+ back here and push more elements to the vector. When the
+ recursion unrolls all such elements will have been popped back
+ already, but if one of those pushes reallocates the vector,
+ previous element references will be invalidated. */
+ parser->scopes.emplace_back ();
+
+ /* Get a reference to the current scope. */
+ scope_level &scope = parser->scopes[parser->scopes.size () - 2];
gdb_xml_debug (parser, _("Entering element <%s>"), name);
@@ -201,7 +231,7 @@ gdb_xml_start_element (void *data, const XML_Char *name,
children. Record that we've seen it. */
seen = 1;
- for (element = scope->elements; element && element->name;
+ for (element = scope.elements; element && element->name;
element++, seen <<= 1)
if (strcmp (element->name, name) == 0)
break;
@@ -213,12 +243,10 @@ gdb_xml_start_element (void *data, const XML_Char *name,
list into the new scope even if there was no match. */
if (parser->is_xinclude)
{
- struct scope_level *unknown_scope;
-
XML_DefaultCurrent (parser->expat_parser);
- unknown_scope = VEC_last (scope_level_s, parser->scopes);
- unknown_scope->elements = scope->elements;
+ scope_level &unknown_scope = parser->scopes.back ();
+ unknown_scope.elements = scope.elements;
return;
}
@@ -226,10 +254,10 @@ gdb_xml_start_element (void *data, const XML_Char *name,
return;
}
- if (!(element->flags & GDB_XML_EF_REPEATABLE) && (seen & scope->seen))
+ if (!(element->flags & GDB_XML_EF_REPEATABLE) && (seen & scope.seen))
gdb_xml_error (parser, _("Element <%s> only expected once"), name);
- scope->seen |= seen;
+ scope.seen |= seen;
back_to = make_cleanup (gdb_xml_values_cleanup, &attributes);
@@ -302,10 +330,13 @@ gdb_xml_start_element (void *data, const XML_Char *name,
if (element->start_handler)
element->start_handler (parser, element, parser->user_data, attributes);
- /* Fill in a new scope level. */
- scope = VEC_last (scope_level_s, parser->scopes);
- scope->element = element;
- scope->elements = element->children;
+ /* Fill in a new scope level. Note that we must delay getting a
+ back reference till here because above we might have recursed,
+ which may have reallocated the vector which invalidates
+ iterators/pointers/references. */
+ scope_level &new_scope = parser->scopes.back ();
+ new_scope.element = element;
+ new_scope.elements = element->children;
do_cleanups (back_to);
}
@@ -342,7 +373,7 @@ gdb_xml_start_element_wrapper (void *data, const XML_Char *name,
static void
gdb_xml_end_element (gdb_xml_parser *parser, const XML_Char *name)
{
- struct scope_level *scope = VEC_last (scope_level_s, parser->scopes);
+ struct scope_level *scope = &parser->scopes.back ();
const struct gdb_xml_element *element;
unsigned int seen;
@@ -387,12 +418,7 @@ gdb_xml_end_element (gdb_xml_parser *parser, const XML_Char *name)
XML_DefaultCurrent (parser->expat_parser);
/* Pop the scope level. */
- if (scope->body)
- {
- obstack_free (scope->body, NULL);
- xfree (scope->body);
- }
- VEC_pop (scope_level_s, parser->scopes);
+ parser->scopes.pop_back ();
}
/* Wrapper for gdb_xml_end_element, to prevent throwing exceptions
@@ -424,19 +450,7 @@ gdb_xml_end_element_wrapper (void *data, const XML_Char *name)
gdb_xml_parser::~gdb_xml_parser ()
{
- struct scope_level *scope;
- int ix;
-
XML_ParserFree (this->expat_parser);
-
- /* Clean up the scopes. */
- for (ix = 0; VEC_iterate (scope_level_s, this->scopes, ix, scope); ix++)
- if (scope->body)
- {
- obstack_free (scope->body, NULL);
- xfree (scope->body);
- }
- VEC_free (scope_level_s, this->scopes);
}
/* Initialize a parser. */
@@ -446,7 +460,6 @@ gdb_xml_parser::gdb_xml_parser (const char *name_,
void *user_data_)
: name (name_),
user_data (user_data_),
- scopes (NULL),
error (exception_none),
last_line (0),
dtd_name (NULL),
@@ -464,10 +477,7 @@ gdb_xml_parser::gdb_xml_parser (const char *name_,
XML_SetCharacterDataHandler (this->expat_parser, gdb_xml_body_text);
/* Initialize the outer scope. */
- scope_level start_scope;
- memset (&start_scope, 0, sizeof (start_scope));
- start_scope.elements = elements;
- VEC_safe_push (scope_level_s, this->scopes, &start_scope);
+ this->scopes.emplace_back (elements);
}
/* External entity handler. The only external entities we support
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/7] xml-support.c: Use std::string for growing string buffer
2017-04-10 12:22 [PATCH 0/7] C++fy xml-support.c and more gdb::optional Pedro Alves
2017-04-10 12:22 ` [PATCH 6/7] gdb_xml_parser: make data fields private and make more functions methods Pedro Alves
2017-04-10 12:22 ` [PATCH 2/7] xml-support.c: Use std::vector Pedro Alves
@ 2017-04-10 12:22 ` Pedro Alves
2017-04-15 4:36 ` Simon Marchi
2017-04-10 12:22 ` [PATCH 3/7] More gdb::optional features Pedro Alves
` (4 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2017-04-10 12:22 UTC (permalink / raw)
To: gdb-patches
This main idea behind this patch is this change to xml-support.c:scope_level
- /* Body text accumulation. This is an owning pointer. */
- struct obstack *body;
+ /* Body text accumulation. */
+ std::string body;
... which allows simplifying other parts of the code.
In target_fetch_description_xml, we want to distinguish between
returning "success + empty std::string" and "no success", and
gdb::optional is a natural fit for that.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* tracefile-tfile.c (tfile_write_tdesc): Adjust to use
gdb::optional<std::string>.
* xml-support.c: Include <string>.
(scope_level::scope_level(scope_level &&))
(scope_level::~scope_level): Delete.
(scope_level::body): Now a std::string.
(gdb_xml_body_text, gdb_xml_end_element): Adjust.
(xinclude_parsing_data::xinclude_parsing_data): Add 'output'
parameter.
(xinclude_parsing_data::~xinclude_parsing_data): Delete.
(xinclude_parsing_data::output): Now a std::string reference.
(xinclude_start_include): Adjust.
(xml_xinclude_default): Adjust.
(xml_process_xincludes): Add 'output' parameter, and return bool.
* xml-support.h (xml_process_xincludes): Add 'output' parameter,
and return bool.
* xml-tdesc.c: Include <unordered_map> and <string>.
(tdesc_xml_cache): Delete.
(tdesc_xml_cache_s): Delete.
(xml_cache): Now an std::unordered_map.
(tdesc_parse_xml): Adjust to use std::string and unordered_map.
(target_fetch_description_xml): Change return type to
gdb::optional<std::string>, and adjust.
* xml-tdesc.h: Include "common/gdb_optional.h" and <string>.
(target_fetch_description_xml): Change return type to
gdb::optional<std::string>.
---
gdb/tracefile-tfile.c | 14 +++----
gdb/xml-support.c | 104 +++++++++++++++-----------------------------------
gdb/xml-support.h | 15 ++++----
gdb/xml-tdesc.c | 81 ++++++++++++++-------------------------
gdb/xml-tdesc.h | 17 +++++++--
5 files changed, 88 insertions(+), 143 deletions(-)
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 5d63c16..255bbc9 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -275,17 +275,19 @@ tfile_write_tdesc (struct trace_file_writer *self)
{
struct tfile_trace_file_writer *writer
= (struct tfile_trace_file_writer *) self;
- char *tdesc = target_fetch_description_xml (¤t_target);
- char *ptr = tdesc;
- char *next;
- if (tdesc == NULL)
+ gdb::optional<std::string> tdesc
+ = target_fetch_description_xml (¤t_target);
+
+ if (!tdesc)
return;
+ const char *ptr = tdesc->c_str ();
+
/* Write tdesc line by line, prefixing each line with "tdesc ". */
while (ptr != NULL)
{
- next = strchr (ptr, '\n');
+ const char *next = strchr (ptr, '\n');
if (next != NULL)
{
fprintf (writer->fp, "tdesc %.*s\n", (int) (next - ptr), ptr);
@@ -299,8 +301,6 @@ tfile_write_tdesc (struct trace_file_writer *self)
}
ptr = next;
}
-
- xfree (tdesc);
}
/* This is the implementation of trace_file_write_ops method
diff --git a/gdb/xml-support.c b/gdb/xml-support.c
index b19f81a..91e31d9 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -23,6 +23,7 @@
#include "filestuff.h"
#include "safe-ctype.h"
#include <vector>
+#include <string>
/* Debugging flag. */
static int debug_xml;
@@ -46,29 +47,9 @@ struct scope_level
explicit scope_level (const gdb_xml_element *elements_ = NULL)
: elements (elements_),
element (NULL),
- seen (0),
- body (NULL)
+ seen (0)
{}
- scope_level (scope_level &&other) noexcept
- : elements (other.elements),
- element (other.element),
- seen (other.seen),
- body (other.body)
- {
- if (this != &other)
- other.body = NULL;
- }
-
- ~scope_level ()
- {
- if (this->body)
- {
- obstack_free (this->body, NULL);
- xfree (this->body);
- }
- }
-
/* Elements we allow at this level. */
const struct gdb_xml_element *elements;
@@ -79,8 +60,8 @@ struct scope_level
optional and repeatable checking). */
unsigned int seen;
- /* Body text accumulation. This is an owning pointer. */
- struct obstack *body;
+ /* Body text accumulation. */
+ std::string body;
};
/* The parser itself, and our additional state. */
@@ -120,14 +101,7 @@ gdb_xml_body_text (void *data, const XML_Char *text, int length)
return;
scope_level &scope = parser->scopes.back ();
-
- if (scope.body == NULL)
- {
- scope.body = XCNEW (struct obstack);
- obstack_init (scope.body);
- }
-
- obstack_grow (scope.body, text, length);
+ scope.body.append (text, length);
}
/* Issue a debugging message from one of PARSER's handlers. */
@@ -390,29 +364,27 @@ gdb_xml_end_element (gdb_xml_parser *parser, const XML_Char *name)
/* Call the element processor. */
if (scope->element != NULL && scope->element->end_handler)
{
- const char *scope_body;
+ const char *body;
- if (scope->body == NULL)
- scope_body = "";
+ if (scope->body.empty ())
+ body = "";
else
{
int length;
- length = obstack_object_size (scope->body);
- obstack_1grow (scope->body, '\0');
- char *body = (char *) obstack_finish (scope->body);
+ length = scope->body.size ();
+ body = scope->body.c_str ();
/* Strip leading and trailing whitespace. */
- while (length > 0 && ISSPACE (body[length-1]))
- body[--length] = '\0';
+ while (length > 0 && ISSPACE (body[length - 1]))
+ length--;
+ scope->body.erase (length);
while (*body && ISSPACE (*body))
body++;
-
- scope_body = body;
}
scope->element->end_handler (parser, scope->element, parser->user_data,
- scope_body);
+ body);
}
else if (scope->element == NULL)
XML_DefaultCurrent (parser->expat_parser);
@@ -726,23 +698,18 @@ gdb_xml_parse_attr_enum (struct gdb_xml_parser *parser,
struct xinclude_parsing_data
{
- xinclude_parsing_data (xml_fetch_another fetcher_, void *fetcher_baton_,
+ xinclude_parsing_data (std::string &output_,
+ xml_fetch_another fetcher_, void *fetcher_baton_,
int include_depth_)
- : skip_depth (0),
+ : output (output_),
+ skip_depth (0),
include_depth (include_depth_),
fetcher (fetcher_),
fetcher_baton (fetcher_baton_)
- {
- obstack_init (&this->obstack);
- }
-
- ~xinclude_parsing_data ()
- {
- obstack_free (&this->obstack, NULL);
- }
+ {}
- /* The obstack to build the output in. */
- struct obstack obstack;
+ /* Where the output goes. */
+ std::string &output;
/* A count indicating whether we are in an element whose
children should not be copied to the output, and if so,
@@ -782,15 +749,11 @@ xinclude_start_include (struct gdb_xml_parser *parser,
gdb_xml_error (parser, _("Could not load XML document \"%s\""), href);
back_to = make_cleanup (xfree, text);
- output = xml_process_xincludes (parser->name, text, data->fetcher,
- data->fetcher_baton,
- data->include_depth + 1);
- if (output == NULL)
+ if (!xml_process_xincludes (data->output, parser->name, text, data->fetcher,
+ data->fetcher_baton,
+ data->include_depth + 1))
gdb_xml_error (parser, _("Parsing \"%s\" failed"), href);
- obstack_grow (&data->obstack, output, strlen (output));
- xfree (output);
-
do_cleanups (back_to);
data->skip_depth++;
@@ -821,7 +784,7 @@ xml_xinclude_default (void *data_, const XML_Char *s, int len)
/* Otherwise just add it to the end of the document we're building
up. */
- obstack_grow (&data->obstack, s, len);
+ data->output.append (s, len);
}
static void XMLCALL
@@ -871,14 +834,13 @@ const struct gdb_xml_element xinclude_elements[] = {
/* The main entry point for <xi:include> processing. */
-char *
-xml_process_xincludes (const char *name, const char *text,
+bool
+xml_process_xincludes (std::string &result,
+ const char *name, const char *text,
xml_fetch_another fetcher, void *fetcher_baton,
int depth)
{
- char *result = NULL;
-
- xinclude_parsing_data data (fetcher, fetcher_baton, depth);
+ xinclude_parsing_data data (result, fetcher, fetcher_baton, depth);
gdb_xml_parser parser (name, xinclude_elements, &data);
parser.is_xinclude = true;
@@ -901,16 +863,12 @@ xml_process_xincludes (const char *name, const char *text,
if (gdb_xml_parse (&parser, text) == 0)
{
- obstack_1grow (&data.obstack, '\0');
- result = xstrdup ((const char *) obstack_finish (&data.obstack));
-
if (depth == 0)
gdb_xml_debug (&parser, _("XInclude processing succeeded."));
+ return true;
}
- else
- result = NULL;
- return result;
+ return false;
}
#endif /* HAVE_LIBEXPAT */
\f
diff --git a/gdb/xml-support.h b/gdb/xml-support.h
index de685e2..8248a32 100644
--- a/gdb/xml-support.h
+++ b/gdb/xml-support.h
@@ -54,17 +54,18 @@ extern const char *xml_builtin[][2];
typedef char *(*xml_fetch_another) (const char *href, void *baton);
-/* Return a new string which is the expansion of TEXT after processing
- <xi:include> tags. FETCHER will be called (with FETCHER_BATON) to
- retrieve any new files. DEPTH should be zero on the initial call.
+/* Append the expansion of TEXT after processing <xi:include> tags in
+ RESULT. FETCHER will be called (with FETCHER_BATON) to retrieve
+ any new files. DEPTH should be zero on the initial call.
- On failure, this function uses NAME in a warning and returns NULL.
+ On failure, this function uses NAME in a warning and returns false.
It may throw an exception, but does not for XML parsing
problems. */
-char *xml_process_xincludes (const char *name, const char *text,
- xml_fetch_another fetcher, void *fetcher_baton,
- int depth);
+bool xml_process_xincludes (std::string &result,
+ const char *name, const char *text,
+ xml_fetch_another fetcher, void *fetcher_baton,
+ int depth);
/* Simplified XML parser infrastructure. */
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 522a9ba..daa713f 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -26,6 +26,8 @@
#include "xml-tdesc.h"
#include "osabi.h"
#include "filenames.h"
+#include <unordered_map>
+#include <string>
/* Maximum sizes.
This is just to catch obviously wrong values. */
@@ -64,15 +66,7 @@ tdesc_parse_xml (const char *document, xml_fetch_another fetcher,
then we will create unnecessary duplicate gdbarches. See
gdbarch_list_lookup_by_info. */
-struct tdesc_xml_cache
-{
- const char *xml_document;
- struct target_desc *tdesc;
-};
-typedef struct tdesc_xml_cache tdesc_xml_cache_s;
-DEF_VEC_O(tdesc_xml_cache_s);
-
-static VEC(tdesc_xml_cache_s) *xml_cache;
+static std::unordered_map<std::string, target_desc *> xml_cache;
/* Callback data for target description parsing. */
@@ -631,55 +625,42 @@ static struct target_desc *
tdesc_parse_xml (const char *document, xml_fetch_another fetcher,
void *fetcher_baton)
{
- struct cleanup *back_to, *result_cleanup;
struct tdesc_parsing_data data;
- struct tdesc_xml_cache *cache;
- char *expanded_text;
- int ix;
/* Expand all XInclude directives. */
- expanded_text = xml_process_xincludes (_("target description"),
- document, fetcher, fetcher_baton, 0);
- if (expanded_text == NULL)
+ std::string expanded_text;
+
+ if (!xml_process_xincludes (expanded_text,
+ _("target description"),
+ document, fetcher, fetcher_baton, 0))
{
warning (_("Could not load XML target description; ignoring"));
return NULL;
}
/* Check for an exact match in the list of descriptions we have
- previously parsed. strcmp is a slightly inefficient way to
- do this; an SHA-1 checksum would work as well. */
- for (ix = 0; VEC_iterate (tdesc_xml_cache_s, xml_cache, ix, cache); ix++)
- if (strcmp (cache->xml_document, expanded_text) == 0)
- {
- xfree (expanded_text);
- return cache->tdesc;
- }
-
- back_to = make_cleanup (null_cleanup, NULL);
+ previously parsed. */
+ const auto it = xml_cache.find (expanded_text);
+ if (it != xml_cache.end ())
+ return it->second;
memset (&data, 0, sizeof (struct tdesc_parsing_data));
data.tdesc = allocate_target_description ();
- result_cleanup = make_cleanup_free_target_description (data.tdesc);
- make_cleanup (xfree, expanded_text);
+ struct cleanup *result_cleanup
+ = make_cleanup_free_target_description (data.tdesc);
if (gdb_xml_parse_quick (_("target description"), "gdb-target.dtd",
- tdesc_elements, expanded_text, &data) == 0)
+ tdesc_elements, expanded_text.c_str (), &data) == 0)
{
/* Parsed successfully. */
- struct tdesc_xml_cache new_cache;
-
- new_cache.xml_document = expanded_text;
- new_cache.tdesc = data.tdesc;
- VEC_safe_push (tdesc_xml_cache_s, xml_cache, &new_cache);
+ xml_cache.emplace (std::move (expanded_text), data.tdesc);
discard_cleanups (result_cleanup);
- do_cleanups (back_to);
return data.tdesc;
}
else
{
warning (_("Could not load XML target description; ignoring"));
- do_cleanups (back_to);
+ do_cleanups (result_cleanup);
return NULL;
}
}
@@ -759,7 +740,7 @@ target_read_description_xml (struct target_ops *ops)
includes, but not parsing it. Used to dump whole tdesc
as a single XML file. */
-char *
+gdb::optional<std::string>
target_fetch_description_xml (struct target_ops *ops)
{
#if !defined(HAVE_LIBEXPAT)
@@ -772,28 +753,24 @@ target_fetch_description_xml (struct target_ops *ops)
"disabled at compile time"));
}
- return NULL;
+ return {};
#else
struct target_desc *tdesc;
- char *tdesc_str;
- char *expanded_text;
- struct cleanup *back_to;
- tdesc_str = fetch_available_features_from_target ("target.xml", ops);
+ gdb::unique_xmalloc_ptr<char>
+ tdesc_str (fetch_available_features_from_target ("target.xml", ops));
if (tdesc_str == NULL)
- return NULL;
+ return {};
- back_to = make_cleanup (xfree, tdesc_str);
- expanded_text = xml_process_xincludes (_("target description"),
- tdesc_str,
- fetch_available_features_from_target, ops, 0);
- do_cleanups (back_to);
- if (expanded_text == NULL)
+ std::string output;
+ if (!xml_process_xincludes (output,
+ _("target description"),
+ tdesc_str.get (),
+ fetch_available_features_from_target, ops, 0))
{
warning (_("Could not load XML target description; ignoring"));
- return NULL;
+ return {};
}
-
- return expanded_text;
+ return output;
#endif
}
diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
index 1637a89..2375395 100644
--- a/gdb/xml-tdesc.h
+++ b/gdb/xml-tdesc.h
@@ -19,6 +19,12 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
+#ifndef XML_TDESC_H
+#define XML_TDESC_H
+
+#include "common/gdb_optional.h"
+#include <string>
+
struct target_ops;
struct target_desc;
@@ -32,8 +38,11 @@ const struct target_desc *file_read_description_xml (const char *filename);
const struct target_desc *target_read_description_xml (struct target_ops *);
-/* Fetches an XML target description using OPS, processing
- includes, but not parsing it. Used to dump whole tdesc
- as a single XML file. */
+/* Fetches an XML target description using OPS, processing includes,
+ but not parsing it. Used to dump whole tdesc as a single XML file.
+ Returns the description on success, and a disengaged optional
+ otherwise. */
+gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);
+
+#endif /* XML_TDESC_H */
-char *target_fetch_description_xml (struct target_ops *ops);
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] xml-support.c: Use std::string for growing string buffer
2017-04-10 12:22 ` [PATCH 5/7] xml-support.c: Use std::string for growing string buffer Pedro Alves
@ 2017-04-15 4:36 ` Simon Marchi
2017-04-15 6:03 ` Simon Marchi
0 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2017-04-15 4:36 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 2017-04-10 08:22, Pedro Alves wrote:
> This main idea behind this patch is this change to
> xml-support.c:scope_level
>
> - /* Body text accumulation. This is an owning pointer. */
> - struct obstack *body;
> + /* Body text accumulation. */
> + std::string body;
>
> ... which allows simplifying other parts of the code.
>
> In target_fetch_description_xml, we want to distinguish between
> returning "success + empty std::string" and "no success", and
> gdb::optional is a natural fit for that.
When growing a string like that, isn't it more efficient to use a
stringstream?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] xml-support.c: Use std::string for growing string buffer
2017-04-15 4:36 ` Simon Marchi
@ 2017-04-15 6:03 ` Simon Marchi
0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2017-04-15 6:03 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 2017-04-15 00:36, Simon Marchi wrote:
> On 2017-04-10 08:22, Pedro Alves wrote:
>> This main idea behind this patch is this change to
>> xml-support.c:scope_level
>>
>> - /* Body text accumulation. This is an owning pointer. */
>> - struct obstack *body;
>> + /* Body text accumulation. */
>> + std::string body;
>>
>> ... which allows simplifying other parts of the code.
>>
>> In target_fetch_description_xml, we want to distinguish between
>> returning "success + empty std::string" and "no success", and
>> gdb::optional is a natural fit for that.
>
> When growing a string like that, isn't it more efficient to use a
> stringstream?
Nevermind, I think I got confused with Java, where string objects are
immutable. So every time you do an append, it actually allocates a new
string object. It's therefore good practice to use a StringBuilder in
those cases. The C++ string works similarly to a vector (with
exponential capacity growth), so the number of reallocations is not bad.
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/7] More gdb::optional features
2017-04-10 12:22 [PATCH 0/7] C++fy xml-support.c and more gdb::optional Pedro Alves
` (2 preceding siblings ...)
2017-04-10 12:22 ` [PATCH 5/7] xml-support.c: Use std::string for growing string buffer Pedro Alves
@ 2017-04-10 12:22 ` Pedro Alves
2017-04-18 23:00 ` Pedro Alves
2017-04-10 12:22 ` [PATCH 1/7] C++-ify gdb/xml-support.c a bit to eliminate cleanups Pedro Alves
` (3 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2017-04-10 12:22 UTC (permalink / raw)
To: gdb-patches
The next feature I'm missing most from gdb::optional<T> compared to
std::optional<T> is construction/move/assignment from a T, instead of
having to default construct an gdb::optional and then use
optional::emplace(....).
For example:
gdb::optional<std::string> function ()
{
gdb::optional<std::string> opt;
std::string str;
...
opt.emplace (std::move (str));
return opt;
vs
gdb::optional<std::string> function ()
{
std::string str;
...
return str;
Those copy/move ctor/assign methods weren't initialy implemented
because std::optional supports construction from a type U if U is
convertible to T too, and has rules to decide whether the ctors are
explicit/implicit based on that, and rules for whether the ctor should
be trivial or not, etc., which leads to a much more complicated
implementation.
If we stick to supporting copy/move construction/assignment of/to an
optional<T> from exactly only optional<T> and T, then all that
conversion-related complication disappears, and we still gain
convenience in most use cases.
The patch also makes emplace return a reference to the constructor
object, per C++17 std::optional, and adds a reset method, againt
because std::optional has one and it's trivial to support it. These
two changes are a requirement of the gdb::optional unit testing patch
that will follow.
(I'm making an effort to avoid importing the whole of libstdc++'s
std::optional. If folks would be OK with that, it would certainly
work for me. But I'd prefer doing it as a follow up.)
gdb/ChangeLog:
2017-04-07 Pedro Alves <palves@redhat.com>
* common/gdb_optional.h: Include common/traits.h.
(in_place_t): New type.
(in_place): New constexpr variable.
(optional::optional): Remove member initialization of
m_instantiated.
(optional::optional(in_place_t...)): New constructor.
(optional::~optional): Use reset.
(optional::optional(const optional&)): New.
(optional::optional(const optional&&)): New.
(optional::optional(T &)): New.
(optional::optional(T &&)): New.
(operator::operator=(const optional &)): New.
(operator::operator=(optional &&)): New.
(operator::operator= (const T &))
(operator::operator= (T &&))
(operator::emplace (Args &&... args)): Return a T&. Use reset.
(operator::reset): New.
(operator::m_instantiated):: Add in-class initializer.
* common/traits.h: Include <type_traits>.
(struct And): New types.
---
gdb/common/gdb_optional.h | 132 ++++++++++++++++++++++++++++++++++++++++------
gdb/common/traits.h | 26 +++++++++
2 files changed, 143 insertions(+), 15 deletions(-)
diff --git a/gdb/common/gdb_optional.h b/gdb/common/gdb_optional.h
index ad1119f..c30fa5b 100644
--- a/gdb/common/gdb_optional.h
+++ b/gdb/common/gdb_optional.h
@@ -20,46 +20,141 @@
#ifndef GDB_OPTIONAL_H
#define GDB_OPTIONAL_H
+#include "common/traits.h"
+
namespace gdb
{
+struct in_place_t
+{
+ explicit in_place_t () = default;
+};
+
+constexpr gdb::in_place_t in_place {};
+
/* This class attempts to be a compatible subset of std::optional,
which is slated to be available in C++17. This class optionally
holds an object of some type -- by default it is constructed not
holding an object, but later the object can be "emplaced". This is
similar to using std::unique_ptr, but in-object allocation is
- guaranteed. */
+ guaranteed.
+
+ Unlike std::optional, we currently only support copy/move
+ construction/assignment of an optional<T> from either exactly
+ optional<T> or T. I.e., we don't support copy/move
+ construction/assignment from optional<U> or U, when U is a type
+ convertible to T. Making that work depending on the definitions of
+ T and U is somewhat complicated, and currently the users of this
+ class don't need it. */
+
template<typename T>
class optional
{
public:
constexpr optional ()
- : m_dummy (),
- m_instantiated (false)
+ : m_dummy ()
+ {}
+
+ template<typename... Args>
+ constexpr optional (in_place_t, Args &&... args)
+ : m_item (std::forward<Args> (args)...),
+ m_instantiated (true)
+ {}
+
+ ~optional ()
+ { this->reset (); }
+
+ /* Copy and move constructors. */
+
+ optional (const optional &other)
{
+ if (other.m_instantiated)
+ this->emplace (other.get ());
}
- ~optional ()
+ optional (optional &&other)
+ noexcept(std::is_nothrow_move_constructible<T> ())
+ {
+ if (other.m_instantiated)
+ this->emplace (std::move (other.get ()));
+ }
+
+ constexpr optional (const T &other)
+ : m_item (other),
+ m_instantiated (true)
+ {}
+
+ constexpr optional (T &&other)
+ noexcept (std::is_nothrow_move_constructible<T> ())
+ : m_item (std::move (other)),
+ m_instantiated (true)
+ {}
+
+ /* Assignment operators. */
+
+ optional &
+ operator= (const optional &other)
+ {
+ if (m_instantiated && other.m_instantiated)
+ this->get () = other.get ();
+ else
+ {
+ if (other.m_instantiated)
+ this->emplace (other.get ());
+ else
+ this->reset ();
+ }
+
+ return *this;
+ }
+
+ optional &
+ operator= (optional &&other)
+ noexcept (And<std::is_nothrow_move_constructible<T>,
+ std::is_nothrow_move_assignable<T>> ())
+ {
+ if (m_instantiated && other.m_instantiated)
+ this->get () = std::move (other.get ());
+ else
+ {
+ if (other.m_instantiated)
+ this->emplace (std::move (other.get ()));
+ else
+ this->reset ();
+ }
+ return *this;
+ }
+
+ optional &
+ operator= (const T &other)
{
if (m_instantiated)
- destroy ();
+ this->get () = other;
+ else
+ this->emplace (other);
+ return *this;
}
- /* These aren't deleted in std::optional, but it was simpler to
- delete them here, because currently the users of this class don't
- need them, and making them depend on the definition of T is
- somewhat complicated. */
- optional (const optional &other) = delete;
- optional<T> &operator= (const optional &other) = delete;
+ optional &
+ operator= (T &&other)
+ noexcept (And<std::is_nothrow_move_constructible<T>,
+ std::is_nothrow_move_assignable<T>> ())
+ {
+ if (m_instantiated)
+ this->get () = std::move (other);
+ else
+ this->emplace (std::move (other));
+ return *this;
+ }
template<typename... Args>
- void emplace (Args &&... args)
+ T &emplace (Args &&... args)
{
- if (m_instantiated)
- destroy ();
+ this->reset ();
new (&m_item) T (std::forward<Args>(args)...);
m_instantiated = true;
+ return this->get ();
}
/* Observers. */
@@ -87,6 +182,13 @@ public:
constexpr bool has_value () const noexcept
{ return m_instantiated; }
+ /* 'reset' is a 'safe' operation with no precondition. */
+ void reset () noexcept
+ {
+ if (m_instantiated)
+ this->destroy ();
+ }
+
private:
/* Destroy the object. */
@@ -109,7 +211,7 @@ private:
};
/* True if the object was ever emplaced. */
- bool m_instantiated;
+ bool m_instantiated = false;
};
}
diff --git a/gdb/common/traits.h b/gdb/common/traits.h
index 4b7bac3..c8f29cc 100644
--- a/gdb/common/traits.h
+++ b/gdb/common/traits.h
@@ -18,6 +18,8 @@
#ifndef COMMON_TRAITS_H
#define COMMON_TRAITS_H
+#include <type_traits>
+
namespace gdb {
/* Pre C++14-safe (CWG 1558) version of C++17's std::void_t. See
@@ -29,6 +31,30 @@ struct make_void { typedef void type; };
template<typename... Ts>
using void_t = typename make_void<Ts...>::type;
+/* A few trait helpers, mainly stolen from libstdc++. Uppercase
+ because "and" is a keyword. */
+
+template<typename...>
+struct And;
+
+template<>
+struct And<> : public std::true_type
+{};
+
+template<typename B1>
+struct And<B1> : public B1
+{};
+
+template<typename B1, typename B2>
+struct And<B1, B2>
+ : public std::conditional<B1::value, B2, B1>::type
+{};
+
+template<typename B1, typename B2, typename B3, typename... Bn>
+struct And<B1, B2, B3, Bn...>
+ : public std::conditional<B1::value, And<B2, B3, Bn...>, B1>::type
+{};
+
}
#endif /* COMMON_TRAITS_H */
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/7] More gdb::optional features
2017-04-10 12:22 ` [PATCH 3/7] More gdb::optional features Pedro Alves
@ 2017-04-18 23:00 ` Pedro Alves
0 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2017-04-18 23:00 UTC (permalink / raw)
To: GDB Patches
This is what I pushed. It's the same patch, but with commit
log tweaked to mention the issue with returning gdb::optional
from functions.
From 22796e972f18c5601cecb0251222411a352836b6 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 18 Apr 2017 21:39:24 +0100
Subject: [PATCH] More gdb::optional features
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Currently we can't use gdb::optional<T> as function return type,
because gdb::optional's copy ctor is deleted. For example, with:
gdb::optional<int> function ()
{
gdb::optional<int> opt;
....
return opt;
we get:
src/gdb/foo.c: In function ‘gdb::optional<int> foo()’:
src/gdb/foo.c:75:10: error: use of deleted function ‘gdb::optional<T>::optional(const gdb::optional<T>&) [with T = int]’
return opt;
^
In file included from src/gdb/foo.c:68:0:
src/gdb/common/gdb_optional.h:53:3: note: declared here
optional (const optional &other) = delete;
^
I started by fixing that, and then ran into another missing feature,
also fixed by this patch.
The next feature I'm missing most from gdb::optional<T> compared to
std::optional<T> is construction/move/assignment from a T, instead of
having to default construct an gdb::optional and then use
optional::emplace(....).
For example:
gdb::optional<std::string> function ()
{
gdb::optional<std::string> opt;
std::string str;
...
opt.emplace (std::move (str));
return opt;
vs
gdb::optional<std::string> function ()
{
std::string str;
...
return str;
The copy/move ctor/assign methods weren't initialy implemented because
std::optional supports construction from a type U if U is convertible
to T too, and has rules to decide whether the ctors are
explicit/implicit based on that, and rules for whether the ctor should
be trivial or not, etc., which leads to a much more complicated
implementation.
If we stick to supporting copy/move construction/assignment of/to an
optional<T> from exactly only optional<T> and T, then all that
conversion-related complication disappears, and we still gain
convenience in most use cases.
The patch also makes emplace return a reference to the constructor
object, per C++17 std::optional, and adds a reset method, againt
because std::optional has one and it's trivial to support it. These
two changes are a requirement of the gdb::optional unit testing patch
that will follow.
gdb/ChangeLog:
2017-04-18 Pedro Alves <palves@redhat.com>
* common/gdb_optional.h: Include common/traits.h.
(in_place_t): New type.
(in_place): New constexpr variable.
(optional::optional): Remove member initialization of
m_instantiated.
(optional::optional(in_place_t...)): New constructor.
(optional::~optional): Use reset.
(optional::optional(const optional&)): New.
(optional::optional(const optional&&)): New.
(optional::optional(T &)): New.
(optional::optional(T &&)): New.
(operator::operator=(const optional &)): New.
(operator::operator=(optional &&)): New.
(operator::operator= (const T &))
(operator::operator= (T &&))
(operator::emplace (Args &&... args)): Return a T&. Use reset.
(operator::reset): New.
(operator::m_instantiated):: Add in-class initializer.
* common/traits.h: Include <type_traits>.
(struct And): New types.
---
gdb/ChangeLog | 23 ++++++++
gdb/common/gdb_optional.h | 132 ++++++++++++++++++++++++++++++++++++++++------
gdb/common/traits.h | 26 +++++++++
3 files changed, 166 insertions(+), 15 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 85ab1ae..f000112 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,28 @@
2017-04-18 Pedro Alves <palves@redhat.com>
+ * common/gdb_optional.h: Include common/traits.h.
+ (in_place_t): New type.
+ (in_place): New constexpr variable.
+ (optional::optional): Remove member initialization of
+ m_instantiated.
+ (optional::optional(in_place_t...)): New constructor.
+ (optional::~optional): Use reset.
+ (optional::optional(const optional&)): New.
+ (optional::optional(const optional&&)): New.
+ (optional::optional(T &)): New.
+ (optional::optional(T &&)): New.
+ (operator::operator=(const optional &)): New.
+ (operator::operator=(optional &&)): New.
+ (operator::operator= (const T &))
+ (operator::operator= (T &&))
+ (operator::emplace (Args &&... args)): Return a T&. Use reset.
+ (operator::reset): New.
+ (operator::m_instantiated):: Add in-class initializer.
+ * common/traits.h: Include <type_traits>.
+ (struct And): New types.
+
+2017-04-18 Pedro Alves <palves@redhat.com>
+
* xml-support.c: Include <vector>.
(scope_level::scope_level(const gdb_xml_element *))
(scope_level::scope_level(scope_level&&)): New.
diff --git a/gdb/common/gdb_optional.h b/gdb/common/gdb_optional.h
index ad1119f..c30fa5b 100644
--- a/gdb/common/gdb_optional.h
+++ b/gdb/common/gdb_optional.h
@@ -20,46 +20,141 @@
#ifndef GDB_OPTIONAL_H
#define GDB_OPTIONAL_H
+#include "common/traits.h"
+
namespace gdb
{
+struct in_place_t
+{
+ explicit in_place_t () = default;
+};
+
+constexpr gdb::in_place_t in_place {};
+
/* This class attempts to be a compatible subset of std::optional,
which is slated to be available in C++17. This class optionally
holds an object of some type -- by default it is constructed not
holding an object, but later the object can be "emplaced". This is
similar to using std::unique_ptr, but in-object allocation is
- guaranteed. */
+ guaranteed.
+
+ Unlike std::optional, we currently only support copy/move
+ construction/assignment of an optional<T> from either exactly
+ optional<T> or T. I.e., we don't support copy/move
+ construction/assignment from optional<U> or U, when U is a type
+ convertible to T. Making that work depending on the definitions of
+ T and U is somewhat complicated, and currently the users of this
+ class don't need it. */
+
template<typename T>
class optional
{
public:
constexpr optional ()
- : m_dummy (),
- m_instantiated (false)
+ : m_dummy ()
+ {}
+
+ template<typename... Args>
+ constexpr optional (in_place_t, Args &&... args)
+ : m_item (std::forward<Args> (args)...),
+ m_instantiated (true)
+ {}
+
+ ~optional ()
+ { this->reset (); }
+
+ /* Copy and move constructors. */
+
+ optional (const optional &other)
{
+ if (other.m_instantiated)
+ this->emplace (other.get ());
}
- ~optional ()
+ optional (optional &&other)
+ noexcept(std::is_nothrow_move_constructible<T> ())
+ {
+ if (other.m_instantiated)
+ this->emplace (std::move (other.get ()));
+ }
+
+ constexpr optional (const T &other)
+ : m_item (other),
+ m_instantiated (true)
+ {}
+
+ constexpr optional (T &&other)
+ noexcept (std::is_nothrow_move_constructible<T> ())
+ : m_item (std::move (other)),
+ m_instantiated (true)
+ {}
+
+ /* Assignment operators. */
+
+ optional &
+ operator= (const optional &other)
+ {
+ if (m_instantiated && other.m_instantiated)
+ this->get () = other.get ();
+ else
+ {
+ if (other.m_instantiated)
+ this->emplace (other.get ());
+ else
+ this->reset ();
+ }
+
+ return *this;
+ }
+
+ optional &
+ operator= (optional &&other)
+ noexcept (And<std::is_nothrow_move_constructible<T>,
+ std::is_nothrow_move_assignable<T>> ())
+ {
+ if (m_instantiated && other.m_instantiated)
+ this->get () = std::move (other.get ());
+ else
+ {
+ if (other.m_instantiated)
+ this->emplace (std::move (other.get ()));
+ else
+ this->reset ();
+ }
+ return *this;
+ }
+
+ optional &
+ operator= (const T &other)
{
if (m_instantiated)
- destroy ();
+ this->get () = other;
+ else
+ this->emplace (other);
+ return *this;
}
- /* These aren't deleted in std::optional, but it was simpler to
- delete them here, because currently the users of this class don't
- need them, and making them depend on the definition of T is
- somewhat complicated. */
- optional (const optional &other) = delete;
- optional<T> &operator= (const optional &other) = delete;
+ optional &
+ operator= (T &&other)
+ noexcept (And<std::is_nothrow_move_constructible<T>,
+ std::is_nothrow_move_assignable<T>> ())
+ {
+ if (m_instantiated)
+ this->get () = std::move (other);
+ else
+ this->emplace (std::move (other));
+ return *this;
+ }
template<typename... Args>
- void emplace (Args &&... args)
+ T &emplace (Args &&... args)
{
- if (m_instantiated)
- destroy ();
+ this->reset ();
new (&m_item) T (std::forward<Args>(args)...);
m_instantiated = true;
+ return this->get ();
}
/* Observers. */
@@ -87,6 +182,13 @@ public:
constexpr bool has_value () const noexcept
{ return m_instantiated; }
+ /* 'reset' is a 'safe' operation with no precondition. */
+ void reset () noexcept
+ {
+ if (m_instantiated)
+ this->destroy ();
+ }
+
private:
/* Destroy the object. */
@@ -109,7 +211,7 @@ private:
};
/* True if the object was ever emplaced. */
- bool m_instantiated;
+ bool m_instantiated = false;
};
}
diff --git a/gdb/common/traits.h b/gdb/common/traits.h
index 4b7bac3..c8f29cc 100644
--- a/gdb/common/traits.h
+++ b/gdb/common/traits.h
@@ -18,6 +18,8 @@
#ifndef COMMON_TRAITS_H
#define COMMON_TRAITS_H
+#include <type_traits>
+
namespace gdb {
/* Pre C++14-safe (CWG 1558) version of C++17's std::void_t. See
@@ -29,6 +31,30 @@ struct make_void { typedef void type; };
template<typename... Ts>
using void_t = typename make_void<Ts...>::type;
+/* A few trait helpers, mainly stolen from libstdc++. Uppercase
+ because "and" is a keyword. */
+
+template<typename...>
+struct And;
+
+template<>
+struct And<> : public std::true_type
+{};
+
+template<typename B1>
+struct And<B1> : public B1
+{};
+
+template<typename B1, typename B2>
+struct And<B1, B2>
+ : public std::conditional<B1::value, B2, B1>::type
+{};
+
+template<typename B1, typename B2, typename B3, typename... Bn>
+struct And<B1, B2, B3, Bn...>
+ : public std::conditional<B1::value, And<B2, B3, Bn...>, B1>::type
+{};
+
}
#endif /* COMMON_TRAITS_H */
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/7] C++-ify gdb/xml-support.c a bit to eliminate cleanups
2017-04-10 12:22 [PATCH 0/7] C++fy xml-support.c and more gdb::optional Pedro Alves
` (3 preceding siblings ...)
2017-04-10 12:22 ` [PATCH 3/7] More gdb::optional features Pedro Alves
@ 2017-04-10 12:22 ` Pedro Alves
2017-04-10 12:22 ` [PATCH 7/7] Eliminate obstack_printf Pedro Alves
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2017-04-10 12:22 UTC (permalink / raw)
To: gdb-patches
Basically convert cleanups to destructors in gdb_xml_parser and
xinclude_parsing_data, and then allocate objects of those types on the
stack.
More C++-ification is possible / will follow, but this removes a few
make_cleanup calls already.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* xml-support.c (gdb_xml_parser): Add ctor/dtor. Make is_xinclude
a bool.
(gdb_xml_end_element): Change type of first parameter.
(gdb_xml_cleanup): Rename to ...
(gdb_xml_parser::~gdb_xml_parser): ... this.
(gdb_xml_create_parser_and_cleanup): Delete with ...
(gdb_xml_parser::gdb_xml_parser): ... creation parts factored out
to this new ctor.
(gdb_xml_parse_quick): Create a local gdb_xml_parser instead of
using gdb_xml_create_parser_and_cleanup.
(xinclude_parsing_data): Add ctor/dtor.
(xml_xinclude_cleanup): Delete.
(xml_process_xincludes): Create a local xinclude_parsing_data
instead of heap-allocating one. Create a local gdb_xml_parser
instead of heap-allocating one with
gdb_xml_create_parser_and_cleanup.
---
gdb/xml-support.c | 150 +++++++++++++++++++++++-------------------------------
1 file changed, 63 insertions(+), 87 deletions(-)
diff --git a/gdb/xml-support.c b/gdb/xml-support.c
index ea0657d..d9dc735 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -61,6 +61,11 @@ DEF_VEC_O(scope_level_s);
/* The parser itself, and our additional state. */
struct gdb_xml_parser
{
+ gdb_xml_parser (const char *name,
+ const struct gdb_xml_element *elements,
+ void *user_data);
+ ~gdb_xml_parser();
+
XML_Parser expat_parser; /* The underlying expat parser. */
const char *name; /* Name of this parser. */
@@ -73,7 +78,7 @@ struct gdb_xml_parser
const char *dtd_name; /* The name of the expected / default DTD,
if specified. */
- int is_xinclude; /* Are we the special <xi:include> parser? */
+ bool is_xinclude; /* Are we the special <xi:include> parser? */
};
/* Process some body text. We accumulate the text for later use; it's
@@ -331,13 +336,12 @@ gdb_xml_start_element_wrapper (void *data, const XML_Char *name,
END_CATCH
}
-/* Handle the end of an element. DATA is our local XML parser, and
+/* Handle the end of an element. PARSER is our local XML parser, and
NAME is the current element. */
static void
-gdb_xml_end_element (void *data, const XML_Char *name)
+gdb_xml_end_element (gdb_xml_parser *parser, const XML_Char *name)
{
- struct gdb_xml_parser *parser = (struct gdb_xml_parser *) data;
struct scope_level *scope = VEC_last (scope_level_s, parser->scopes);
const struct gdb_xml_element *element;
unsigned int seen;
@@ -404,7 +408,7 @@ gdb_xml_end_element_wrapper (void *data, const XML_Char *name)
TRY
{
- gdb_xml_end_element (data, name);
+ gdb_xml_end_element (parser, name);
}
CATCH (ex, RETURN_MASK_ALL)
{
@@ -418,66 +422,52 @@ gdb_xml_end_element_wrapper (void *data, const XML_Char *name)
/* Free a parser and all its associated state. */
-static void
-gdb_xml_cleanup (void *arg)
+gdb_xml_parser::~gdb_xml_parser ()
{
- struct gdb_xml_parser *parser = (struct gdb_xml_parser *) arg;
struct scope_level *scope;
int ix;
- XML_ParserFree (parser->expat_parser);
+ XML_ParserFree (this->expat_parser);
/* Clean up the scopes. */
- for (ix = 0; VEC_iterate (scope_level_s, parser->scopes, ix, scope); ix++)
+ for (ix = 0; VEC_iterate (scope_level_s, this->scopes, ix, scope); ix++)
if (scope->body)
{
obstack_free (scope->body, NULL);
xfree (scope->body);
}
- VEC_free (scope_level_s, parser->scopes);
-
- xfree (parser);
+ VEC_free (scope_level_s, this->scopes);
}
-/* Initialize a parser and store it to *PARSER_RESULT. Register a
- cleanup to destroy the parser. */
-
-static struct cleanup *
-gdb_xml_create_parser_and_cleanup (const char *name,
- const struct gdb_xml_element *elements,
- void *user_data,
- struct gdb_xml_parser **parser_result)
+/* Initialize a parser. */
+
+gdb_xml_parser::gdb_xml_parser (const char *name_,
+ const gdb_xml_element *elements,
+ void *user_data_)
+ : name (name_),
+ user_data (user_data_),
+ scopes (NULL),
+ error (exception_none),
+ last_line (0),
+ dtd_name (NULL),
+ is_xinclude (false)
{
- struct gdb_xml_parser *parser;
- struct scope_level start_scope;
- struct cleanup *result;
-
- /* Initialize the parser. */
- parser = XCNEW (struct gdb_xml_parser);
- parser->expat_parser = XML_ParserCreateNS (NULL, '!');
- if (parser->expat_parser == NULL)
- {
- xfree (parser);
- malloc_failure (0);
- }
-
- parser->name = name;
+ this->expat_parser = XML_ParserCreateNS (NULL, '!');
+ if (this->expat_parser == NULL)
+ malloc_failure (0);
- parser->user_data = user_data;
- XML_SetUserData (parser->expat_parser, parser);
+ XML_SetUserData (this->expat_parser, this);
/* Set the callbacks. */
- XML_SetElementHandler (parser->expat_parser, gdb_xml_start_element_wrapper,
+ XML_SetElementHandler (this->expat_parser, gdb_xml_start_element_wrapper,
gdb_xml_end_element_wrapper);
- XML_SetCharacterDataHandler (parser->expat_parser, gdb_xml_body_text);
+ XML_SetCharacterDataHandler (this->expat_parser, gdb_xml_body_text);
/* Initialize the outer scope. */
+ scope_level start_scope;
memset (&start_scope, 0, sizeof (start_scope));
start_scope.elements = elements;
- VEC_safe_push (scope_level_s, parser->scopes, &start_scope);
-
- *parser_result = parser;
- return make_cleanup (gdb_xml_cleanup, parser);
+ VEC_safe_push (scope_level_s, this->scopes, &start_scope);
}
/* External entity handler. The only external entities we support
@@ -603,19 +593,10 @@ gdb_xml_parse_quick (const char *name, const char *dtd_name,
const struct gdb_xml_element *elements,
const char *document, void *user_data)
{
- struct gdb_xml_parser *parser;
- struct cleanup *back_to;
- int result;
-
- back_to = gdb_xml_create_parser_and_cleanup (name, elements,
- user_data, &parser);
+ gdb_xml_parser parser (name, elements, user_data);
if (dtd_name != NULL)
- gdb_xml_use_dtd (parser, dtd_name);
- result = gdb_xml_parse (parser, document);
-
- do_cleanups (back_to);
-
- return result;
+ gdb_xml_use_dtd (&parser, dtd_name);
+ return gdb_xml_parse (&parser, document);
}
/* Parse a field VALSTR that we expect to contain an integer value.
@@ -735,6 +716,21 @@ gdb_xml_parse_attr_enum (struct gdb_xml_parser *parser,
struct xinclude_parsing_data
{
+ xinclude_parsing_data (xml_fetch_another fetcher_, void *fetcher_baton_,
+ int include_depth_)
+ : skip_depth (0),
+ include_depth (include_depth_),
+ fetcher (fetcher_),
+ fetcher_baton (fetcher_baton_)
+ {
+ obstack_init (&this->obstack);
+ }
+
+ ~xinclude_parsing_data ()
+ {
+ obstack_free (&this->obstack, NULL);
+ }
+
/* The obstack to build the output in. */
struct obstack obstack;
@@ -851,15 +847,6 @@ xml_xinclude_xml_decl (void *data_, const XML_Char *version,
output. */
}
-static void
-xml_xinclude_cleanup (void *data_)
-{
- struct xinclude_parsing_data *data = (struct xinclude_parsing_data *) data_;
-
- obstack_free (&data->obstack, NULL);
- xfree (data);
-}
-
const struct gdb_xml_attribute xinclude_attributes[] = {
{ "href", GDB_XML_AF_NONE, NULL, NULL },
{ NULL, GDB_XML_AF_NONE, NULL, NULL }
@@ -879,51 +866,40 @@ xml_process_xincludes (const char *name, const char *text,
xml_fetch_another fetcher, void *fetcher_baton,
int depth)
{
- struct gdb_xml_parser *parser;
- struct xinclude_parsing_data *data;
- struct cleanup *back_to;
char *result = NULL;
- data = XCNEW (struct xinclude_parsing_data);
- obstack_init (&data->obstack);
- back_to = make_cleanup (xml_xinclude_cleanup, data);
-
- gdb_xml_create_parser_and_cleanup (name, xinclude_elements,
- data, &parser);
- parser->is_xinclude = 1;
+ xinclude_parsing_data data (fetcher, fetcher_baton, depth);
- data->include_depth = depth;
- data->fetcher = fetcher;
- data->fetcher_baton = fetcher_baton;
+ gdb_xml_parser parser (name, xinclude_elements, &data);
+ parser.is_xinclude = true;
- XML_SetCharacterDataHandler (parser->expat_parser, NULL);
- XML_SetDefaultHandler (parser->expat_parser, xml_xinclude_default);
+ XML_SetCharacterDataHandler (parser.expat_parser, NULL);
+ XML_SetDefaultHandler (parser.expat_parser, xml_xinclude_default);
/* Always discard the XML version declarations; the only important
thing this provides is encoding, and our result will have been
converted to UTF-8. */
- XML_SetXmlDeclHandler (parser->expat_parser, xml_xinclude_xml_decl);
+ XML_SetXmlDeclHandler (parser.expat_parser, xml_xinclude_xml_decl);
if (depth > 0)
/* Discard the doctype for included documents. */
- XML_SetDoctypeDeclHandler (parser->expat_parser,
+ XML_SetDoctypeDeclHandler (parser.expat_parser,
xml_xinclude_start_doctype,
xml_xinclude_end_doctype);
- gdb_xml_use_dtd (parser, "xinclude.dtd");
+ gdb_xml_use_dtd (&parser, "xinclude.dtd");
- if (gdb_xml_parse (parser, text) == 0)
+ if (gdb_xml_parse (&parser, text) == 0)
{
- obstack_1grow (&data->obstack, '\0');
- result = xstrdup ((const char *) obstack_finish (&data->obstack));
+ obstack_1grow (&data.obstack, '\0');
+ result = xstrdup ((const char *) obstack_finish (&data.obstack));
if (depth == 0)
- gdb_xml_debug (parser, _("XInclude processing succeeded."));
+ gdb_xml_debug (&parser, _("XInclude processing succeeded."));
}
else
result = NULL;
- do_cleanups (back_to);
return result;
}
#endif /* HAVE_LIBEXPAT */
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 7/7] Eliminate obstack_printf
2017-04-10 12:22 [PATCH 0/7] C++fy xml-support.c and more gdb::optional Pedro Alves
` (4 preceding siblings ...)
2017-04-10 12:22 ` [PATCH 1/7] C++-ify gdb/xml-support.c a bit to eliminate cleanups Pedro Alves
@ 2017-04-10 12:22 ` Pedro Alves
2017-04-10 12:32 ` [PATCH 4/7] gdb::optional unit tests Pedro Alves
2017-04-15 16:20 ` [PATCH 0/7] C++fy xml-support.c and more gdb::optional Simon Marchi
7 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2017-04-10 12:22 UTC (permalink / raw)
To: gdb-patches
Not used anywhere.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* xml-support.c (obstack_xml_printf): Delete.
* xml-support.h (obstack_xml_printf): Delete.
---
gdb/xml-support.c | 40 ----------------------------------------
gdb/xml-support.h | 7 -------
2 files changed, 47 deletions(-)
diff --git a/gdb/xml-support.c b/gdb/xml-support.c
index 9595246..6ca1630 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -997,46 +997,6 @@ show_debug_xml (struct ui_file *file, int from_tty,
fprintf_filtered (file, _("XML debugging is %s.\n"), value);
}
-void
-obstack_xml_printf (struct obstack *obstack, const char *format, ...)
-{
- va_list ap;
- const char *f;
- const char *prev;
- int percent = 0;
-
- va_start (ap, format);
-
- prev = format;
- for (f = format; *f; f++)
- {
- if (percent)
- {
- switch (*f)
- {
- case 's':
- {
- char *p;
- char *a = va_arg (ap, char *);
-
- obstack_grow (obstack, prev, f - prev - 1);
- p = xml_escape_text (a);
- obstack_grow_str (obstack, p);
- xfree (p);
- prev = f + 1;
- }
- break;
- }
- percent = 0;
- }
- else if (*f == '%')
- percent = 1;
- }
-
- obstack_grow_str (obstack, prev);
- va_end (ap);
-}
-
char *
xml_fetch_content_from_file (const char *filename, void *baton)
{
diff --git a/gdb/xml-support.h b/gdb/xml-support.h
index f9ea64d..1a1b7fd 100644
--- a/gdb/xml-support.h
+++ b/gdb/xml-support.h
@@ -227,13 +227,6 @@ extern gdb_xml_attribute_handler gdb_xml_parse_attr_enum;
ULONGEST gdb_xml_parse_ulongest (struct gdb_xml_parser *parser,
const char *value);
-/* Simple printf to obstack function. Current implemented formatters:
- %s - grow an xml escaped text in OBSTACK. */
-
-extern void obstack_xml_printf (struct obstack *obstack,
- const char *format, ...)
- ATTRIBUTE_PRINTF_2;
-
/* Open FILENAME, read all its text into memory, close it, and return
the text. If something goes wrong, return NULL and warn. */
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/7] gdb::optional unit tests
2017-04-10 12:22 [PATCH 0/7] C++fy xml-support.c and more gdb::optional Pedro Alves
` (5 preceding siblings ...)
2017-04-10 12:22 ` [PATCH 7/7] Eliminate obstack_printf Pedro Alves
@ 2017-04-10 12:32 ` Pedro Alves
2017-04-15 16:20 ` [PATCH 0/7] C++fy xml-support.c and more gdb::optional Simon Marchi
7 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2017-04-10 12:32 UTC (permalink / raw)
To: gdb-patches
I thought I'd add some unit tests to make sure gdb::optional behaved
correctly, and started writing some, but then thought/realized that
libstdc++ already has extensive testing for C++17 std::optional, which
gdb::optional is a subset of, and thought why bother writing something
from scratch. So I tried copying over a subset of libstdc++'s tests
(that ones that cover the subset supported by gdb::optional), and was
positively surprised that they mostly work OOTB. This did help shake
out a few bugs from what I was implementing in the previous patch to
gdb::optional. Still, it's a good chunk of code being copied over, so
if people dislike this copying/duplication, I can drop this patch.
gdb/ChangeLog:
2017-04-07 Pedro Alves <palves@redhat.com>
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/optional-selftests.c.
(SUBDIR_UNITTESTS_OBS): Add optional-selftests.o.
* unittests/optional-selftests.c: New file.
* unittests/optional/assignment/1.cc: New file.
* unittests/optional/assignment/2.cc: New file.
* unittests/optional/assignment/3.cc: New file.
* unittests/optional/assignment/4.cc: New file.
* unittests/optional/assignment/5.cc: New file.
* unittests/optional/assignment/6.cc: New file.
* unittests/optional/assignment/7.cc: New file.
* unittests/optional/cons/copy.cc: New file.
* unittests/optional/cons/default.cc: New file.
* unittests/optional/cons/move.cc: New file.
* unittests/optional/cons/value.cc: New file.
* unittests/optional/in_place.cc: New file.
* unittests/optional/observers/1.cc: New file.
* unittests/optional/observers/2.cc: New file.
---
gdb/Makefile.in | 6 +-
gdb/unittests/optional-selftests.c | 94 +++++++++++
gdb/unittests/optional/assignment/1.cc | 195 ++++++++++++++++++++++
gdb/unittests/optional/assignment/2.cc | 193 ++++++++++++++++++++++
gdb/unittests/optional/assignment/3.cc | 156 +++++++++++++++++
gdb/unittests/optional/assignment/4.cc | 156 +++++++++++++++++
gdb/unittests/optional/assignment/5.cc | 80 +++++++++
gdb/unittests/optional/assignment/6.cc | 90 ++++++++++
gdb/unittests/optional/assignment/7.cc | 29 ++++
gdb/unittests/optional/cons/copy.cc | 126 ++++++++++++++
gdb/unittests/optional/cons/default.cc | 58 +++++++
gdb/unittests/optional/cons/move.cc | 124 ++++++++++++++
gdb/unittests/optional/cons/value.cc | 294 +++++++++++++++++++++++++++++++++
gdb/unittests/optional/in_place.cc | 65 ++++++++
gdb/unittests/optional/observers/1.cc | 31 ++++
gdb/unittests/optional/observers/2.cc | 35 ++++
16 files changed, 1730 insertions(+), 2 deletions(-)
create mode 100644 gdb/unittests/optional-selftests.c
create mode 100644 gdb/unittests/optional/assignment/1.cc
create mode 100644 gdb/unittests/optional/assignment/2.cc
create mode 100644 gdb/unittests/optional/assignment/3.cc
create mode 100644 gdb/unittests/optional/assignment/4.cc
create mode 100644 gdb/unittests/optional/assignment/5.cc
create mode 100644 gdb/unittests/optional/assignment/6.cc
create mode 100644 gdb/unittests/optional/assignment/7.cc
create mode 100644 gdb/unittests/optional/cons/copy.cc
create mode 100644 gdb/unittests/optional/cons/default.cc
create mode 100644 gdb/unittests/optional/cons/move.cc
create mode 100644 gdb/unittests/optional/cons/value.cc
create mode 100644 gdb/unittests/optional/in_place.cc
create mode 100644 gdb/unittests/optional/observers/1.cc
create mode 100644 gdb/unittests/optional/observers/2.cc
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 23e4bed..92dbff0 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -526,12 +526,14 @@ SUBDIR_PYTHON_CFLAGS =
SUBDIR_UNITTESTS_SRCS = \
unittests/function-view-selftests.c \
unittests/offset-type-selftests.c \
- unittests/ptid-selftests.c
+ unittests/ptid-selftests.c \
+ unittests/optional-selftests.c
SUBDIR_UNITTESTS_OBS = \
function-view-selftests.o \
offset-type-selftests.o \
- ptid-selftests.o
+ ptid-selftests.o \
+ optional-selftests.o
# Opcodes currently live in one of two places. Either they are in the
# opcode library, typically ../opcodes, or they are in a header file
diff --git a/gdb/unittests/optional-selftests.c b/gdb/unittests/optional-selftests.c
new file mode 100644
index 0000000..76343c6
--- /dev/null
+++ b/gdb/unittests/optional-selftests.c
@@ -0,0 +1,94 @@
+/* Self tests for optional for GDB, the GNU debugger.
+
+ Copyright (C) 2017 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "selftest.h"
+#include "common/gdb_optional.h"
+
+/* Used by the included .cc files below. Included here because the
+ included test files are wrapped in a namespace. */
+#include <vector>
+#include <string>
+#include <memory>
+
+/* libstdc++'s testsuite uses VERIFY. */
+#define VERIFY SELF_CHECK
+
+/* Used to disable testing features not supported by
+ gdb::optional. */
+#define GDB_OPTIONAL
+
+namespace selftests {
+namespace optional {
+
+/* The actual tests live in separate files, which were originally
+ copied over from libstdc++'s testsuite. To preserve the structure
+ and help with comparison with the original tests, the file names
+ have been preserved, and only minimal modification was done to have
+ them compile against gdb::optional instead of std::optional:
+
+ - std::optional->gdb:optional, etc.
+ - ATTRIBUTE_UNUSED in a few places
+ - wrap each file in a namespace so they can all be compiled as a
+ single unit.
+ - libstdc++'s license and formatting style was preserved.
+*/
+
+#include "optional/assignment/1.cc"
+#include "optional/assignment/2.cc"
+#include "optional/assignment/3.cc"
+#include "optional/assignment/4.cc"
+#include "optional/assignment/5.cc"
+#include "optional/assignment/6.cc"
+#include "optional/assignment/7.cc"
+#include "optional/cons/copy.cc"
+#include "optional/cons/default.cc"
+#include "optional/cons/move.cc"
+#include "optional/cons/value.cc"
+#include "optional/in_place.cc"
+#include "optional/observers/1.cc"
+#include "optional/observers/2.cc"
+
+static void
+run_tests ()
+{
+ assign_1::test ();
+ assign_2::test ();
+ assign_3::test ();
+ assign_4::test ();
+ assign_5::test ();
+ assign_6::test ();
+ assign_7::test ();
+ cons_copy::test ();
+ cons_default::test ();
+ cons_move::test ();
+ cons_value::test ();
+ in_place::test ();
+ observers_1::test ();
+ observers_2::test ();
+}
+
+} /* namespace optional */
+} /* namespace selftests */
+
+void
+_initialize_optional_selftests ()
+{
+ register_self_test (selftests::optional::run_tests);
+}
diff --git a/gdb/unittests/optional/assignment/1.cc b/gdb/unittests/optional/assignment/1.cc
new file mode 100644
index 0000000..671004e
--- /dev/null
+++ b/gdb/unittests/optional/assignment/1.cc
@@ -0,0 +1,195 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace assign_1 {
+
+struct exception {};
+
+int counter = 0;
+
+struct mixin_counter
+{
+ mixin_counter() { ++counter; }
+ mixin_counter(mixin_counter const&) { ++counter; }
+ ~mixin_counter() { --counter; }
+};
+
+struct value_type : private mixin_counter
+{
+ enum state_type
+ {
+ zero,
+ moved_from,
+ throwing_construction,
+ throwing_copy,
+ throwing_copy_assignment,
+ throwing_move,
+ throwing_move_assignment,
+ threw,
+ };
+
+ value_type() = default;
+
+ explicit value_type(state_type state_)
+ : state(state_)
+ {
+ throw_if(throwing_construction);
+ }
+
+ value_type(value_type const& other)
+ : state(other.state)
+ {
+ throw_if(throwing_copy);
+ }
+
+ value_type&
+ operator=(value_type const& other)
+ {
+ state = other.state;
+ throw_if(throwing_copy_assignment);
+ return *this;
+ }
+
+ value_type(value_type&& other)
+ : state(other.state)
+ {
+ other.state = moved_from;
+ throw_if(throwing_move);
+ }
+
+ value_type&
+ operator=(value_type&& other)
+ {
+ state = other.state;
+ other.state = moved_from;
+ throw_if(throwing_move_assignment);
+ return *this;
+ }
+
+ void throw_if(state_type match)
+ {
+ if(state == match)
+ {
+ state = threw;
+ throw exception {};
+ }
+ }
+
+ state_type state = zero;
+};
+
+void test()
+{
+ using O = gdb::optional<value_type>;
+ using S = value_type::state_type;
+ auto const make = [](S s = S::zero) { return O { gdb::in_place, s }; };
+
+ enum outcome_type { nothrow, caught, bad_catch };
+
+ // Check copy/move assignment for disengaged optional
+
+ // From disengaged optional
+ {
+ O o;
+ VERIFY( !o );
+ O p;
+ o = p;
+ VERIFY( !o );
+ VERIFY( !p );
+ }
+
+ {
+ O o;
+ VERIFY( !o );
+ O p;
+ o = std::move(p);
+ VERIFY( !o );
+ VERIFY( !p );
+ }
+
+#ifndef GDB_OPTIONAL
+ {
+ O o;
+ VERIFY( !o );
+ o = {};
+ VERIFY( !o );
+ }
+#endif
+
+ // From engaged optional
+ {
+ O o;
+ VERIFY( !o );
+ O p = make(S::throwing_copy_assignment);
+ o = p;
+ VERIFY( o && o->state == S::throwing_copy_assignment );
+ VERIFY( p && p->state == S::throwing_copy_assignment );
+ }
+
+ {
+ O o;
+ VERIFY( !o );
+ O p = make(S::throwing_move_assignment);
+ o = std::move(p);
+ VERIFY( o && o->state == S::throwing_move_assignment );
+ VERIFY( p && p->state == S::moved_from );
+ }
+
+ {
+ outcome_type outcome {};
+ O o;
+ VERIFY( !o );
+ O p = make(S::throwing_copy);
+
+ try
+ {
+ o = p;
+ }
+ catch(exception const&)
+ { outcome = caught; }
+ catch(...)
+ { outcome = bad_catch; }
+
+ VERIFY( outcome == caught );
+ VERIFY( !o );
+ VERIFY( p && p->state == S::throwing_copy );
+ }
+
+ {
+ outcome_type outcome {};
+ O o;
+ VERIFY( !o );
+ O p = make(S::throwing_move);
+
+ try
+ {
+ o = std::move(p);
+ }
+ catch(exception const&)
+ { outcome = caught; }
+ catch(...)
+ { outcome = bad_catch; }
+
+ VERIFY( outcome == caught );
+ VERIFY( !o );
+ VERIFY( p && p->state == S::moved_from );
+ }
+
+ VERIFY( counter == 0 );
+}
+
+} // namespace assign_1
diff --git a/gdb/unittests/optional/assignment/2.cc b/gdb/unittests/optional/assignment/2.cc
new file mode 100644
index 0000000..1b0bd7a
--- /dev/null
+++ b/gdb/unittests/optional/assignment/2.cc
@@ -0,0 +1,193 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace assign_2 {
+
+struct exception {};
+
+int counter = 0;
+
+struct mixin_counter
+{
+ mixin_counter() { ++counter; }
+ mixin_counter(mixin_counter const&) { ++counter; }
+ ~mixin_counter() { --counter; }
+};
+
+struct value_type : private mixin_counter
+{
+ enum state_type
+ {
+ zero,
+ moved_from,
+ throwing_construction,
+ throwing_copy,
+ throwing_copy_assignment,
+ throwing_move,
+ throwing_move_assignment,
+ threw,
+ };
+
+ value_type() = default;
+
+ explicit value_type(state_type state_)
+ : state(state_)
+ {
+ throw_if(throwing_construction);
+ }
+
+ value_type(value_type const& other)
+ : state(other.state)
+ {
+ throw_if(throwing_copy);
+ }
+
+ value_type&
+ operator=(value_type const& other)
+ {
+ state = other.state;
+ throw_if(throwing_copy_assignment);
+ return *this;
+ }
+
+ value_type(value_type&& other)
+ : state(other.state)
+ {
+ other.state = moved_from;
+ throw_if(throwing_move);
+ }
+
+ value_type&
+ operator=(value_type&& other)
+ {
+ state = other.state;
+ other.state = moved_from;
+ throw_if(throwing_move_assignment);
+ return *this;
+ }
+
+ void throw_if(state_type match)
+ {
+ if(state == match)
+ {
+ state = threw;
+ throw exception {};
+ }
+ }
+
+ state_type state = zero;
+};
+
+void test()
+{
+ using O = gdb::optional<value_type>;
+ using S = value_type::state_type;
+ auto const make = [](S s = S::zero) { return O { gdb::in_place, s }; };
+
+ enum outcome_type { nothrow, caught, bad_catch };
+
+ // Check copy/move assignment for engaged optional
+
+ // From disengaged optional
+ {
+ O o = make(S::zero);
+ VERIFY( o );
+ O p;
+ o = p;
+ VERIFY( !o );
+ VERIFY( !p );
+ }
+
+ {
+ O o = make(S::zero);
+ VERIFY( o );
+ O p;
+ o = std::move(p);
+ VERIFY( !o );
+ VERIFY( !p );
+ }
+
+#ifndef GDB_OPTIONAL
+ {
+ O o = make(S::zero);
+ VERIFY( o );
+ o = {};
+ VERIFY( !o );
+ }
+#endif
+
+ // From engaged optional
+ {
+ O o = make(S::zero);
+ VERIFY( o );
+ O p = make(S::throwing_copy);
+ o = p;
+ VERIFY( o && o->state == S::throwing_copy);
+ VERIFY( p && p->state == S::throwing_copy);
+ }
+
+ {
+ O o = make(S::zero);
+ VERIFY( o );
+ O p = make(S::throwing_move);
+ o = std::move(p);
+ VERIFY( o && o->state == S::throwing_move);
+ VERIFY( p && p->state == S::moved_from);
+ }
+
+ {
+ ATTRIBUTE_UNUSED outcome_type outcome {};
+ O o = make(S::zero);
+ VERIFY( o );
+ O p = make(S::throwing_copy_assignment);
+
+ try
+ {
+ o = p;
+ }
+ catch(exception const&)
+ { outcome = caught; }
+ catch(...)
+ { outcome = bad_catch; }
+
+ VERIFY( o && o->state == S::threw);
+ VERIFY( p && p->state == S::throwing_copy_assignment);
+ }
+
+ {
+ ATTRIBUTE_UNUSED outcome_type outcome {};
+ O o = make(S::zero);
+ VERIFY( o );
+ O p = make(S::throwing_move_assignment);
+
+ try
+ {
+ o = std::move(p);
+ }
+ catch(exception const&)
+ { outcome = caught; }
+ catch(...)
+ { outcome = bad_catch; }
+
+ VERIFY( o && o->state == S::threw);
+ VERIFY( p && p->state == S::moved_from);
+ }
+
+ VERIFY( counter == 0 );
+}
+
+} // namespace assign_2
diff --git a/gdb/unittests/optional/assignment/3.cc b/gdb/unittests/optional/assignment/3.cc
new file mode 100644
index 0000000..e047e74
--- /dev/null
+++ b/gdb/unittests/optional/assignment/3.cc
@@ -0,0 +1,156 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace assign_3 {
+
+struct exception {};
+
+int counter = 0;
+
+struct mixin_counter
+{
+ mixin_counter() { ++counter; }
+ mixin_counter(mixin_counter const&) { ++counter; }
+ ~mixin_counter() { --counter; }
+};
+
+struct value_type : private mixin_counter
+{
+ enum state_type
+ {
+ zero,
+ moved_from,
+ throwing_construction,
+ throwing_copy,
+ throwing_copy_assignment,
+ throwing_move,
+ throwing_move_assignment,
+ threw,
+ };
+
+ value_type() = default;
+
+ explicit value_type(state_type state_)
+ : state(state_)
+ {
+ throw_if(throwing_construction);
+ }
+
+ value_type(value_type const& other)
+ : state(other.state)
+ {
+ throw_if(throwing_copy);
+ }
+
+ value_type&
+ operator=(value_type const& other)
+ {
+ state = other.state;
+ throw_if(throwing_copy_assignment);
+ return *this;
+ }
+
+ value_type(value_type&& other)
+ : state(other.state)
+ {
+ other.state = moved_from;
+ throw_if(throwing_move);
+ }
+
+ value_type&
+ operator=(value_type&& other)
+ {
+ state = other.state;
+ other.state = moved_from;
+ throw_if(throwing_move_assignment);
+ return *this;
+ }
+
+ void throw_if(state_type match)
+ {
+ if(state == match)
+ {
+ state = threw;
+ throw exception {};
+ }
+ }
+
+ state_type state = zero;
+};
+
+void test()
+{
+ using O = gdb::optional<value_type>;
+ using S = value_type::state_type;
+ auto const make = [](S s = S::zero) { return value_type { s }; };
+
+ enum outcome_type { nothrow, caught, bad_catch };
+
+ // Check value assignment for disengaged optional
+
+ {
+ O o;
+ value_type v = make(S::throwing_copy_assignment);
+ o = v;
+ VERIFY( o && o->state == S::throwing_copy_assignment );
+ }
+
+ {
+ O o;
+ value_type v = make(S::throwing_move_assignment);
+ o = std::move(v);
+ VERIFY( o && o->state == S::throwing_move_assignment );
+ }
+
+ {
+ ATTRIBUTE_UNUSED outcome_type outcome {};
+ O o;
+ value_type v = make(S::throwing_copy);
+
+ try
+ {
+ o = v;
+ }
+ catch(exception const&)
+ { outcome = caught; }
+ catch(...)
+ { outcome = bad_catch; }
+
+ VERIFY( !o );
+ }
+
+ {
+ ATTRIBUTE_UNUSED outcome_type outcome {};
+ O o;
+ value_type v = make(S::throwing_move);
+
+ try
+ {
+ o = std::move(v);
+ }
+ catch(exception const&)
+ { outcome = caught; }
+ catch(...)
+ { outcome = bad_catch; }
+
+ VERIFY( !o );
+ }
+
+ VERIFY( counter == 0 );
+}
+
+} // namespace assign_3
diff --git a/gdb/unittests/optional/assignment/4.cc b/gdb/unittests/optional/assignment/4.cc
new file mode 100644
index 0000000..0b196e0
--- /dev/null
+++ b/gdb/unittests/optional/assignment/4.cc
@@ -0,0 +1,156 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace assign_4 {
+
+struct exception {};
+
+int counter = 0;
+
+struct mixin_counter
+{
+ mixin_counter() { ++counter; }
+ mixin_counter(mixin_counter const&) { ++counter; }
+ ~mixin_counter() { --counter; }
+};
+
+struct value_type : private mixin_counter
+{
+ enum state_type
+ {
+ zero,
+ moved_from,
+ throwing_construction,
+ throwing_copy,
+ throwing_copy_assignment,
+ throwing_move,
+ throwing_move_assignment,
+ threw,
+ };
+
+ value_type() = default;
+
+ explicit value_type(state_type state_)
+ : state(state_)
+ {
+ throw_if(throwing_construction);
+ }
+
+ value_type(value_type const& other)
+ : state(other.state)
+ {
+ throw_if(throwing_copy);
+ }
+
+ value_type&
+ operator=(value_type const& other)
+ {
+ state = other.state;
+ throw_if(throwing_copy_assignment);
+ return *this;
+ }
+
+ value_type(value_type&& other)
+ : state(other.state)
+ {
+ other.state = moved_from;
+ throw_if(throwing_move);
+ }
+
+ value_type&
+ operator=(value_type&& other)
+ {
+ state = other.state;
+ other.state = moved_from;
+ throw_if(throwing_move_assignment);
+ return *this;
+ }
+
+ void throw_if(state_type match)
+ {
+ if(state == match)
+ {
+ state = threw;
+ throw exception {};
+ }
+ }
+
+ state_type state = zero;
+};
+
+void test()
+{
+ using O = gdb::optional<value_type>;
+ using S = value_type::state_type;
+ auto const make = [](S s = S::zero) { return value_type { s }; };
+
+ enum outcome_type { nothrow, caught, bad_catch };
+
+ // Check value assignment for engaged optional
+
+ {
+ O o = make();
+ value_type v = make(S::throwing_copy);
+ o = v;
+ VERIFY( o && o->state == S::throwing_copy);
+ }
+
+ {
+ O o = make();
+ value_type v = make(S::throwing_move);
+ o = std::move(v);
+ VERIFY( o && o->state == S::throwing_move);
+ }
+
+ {
+ ATTRIBUTE_UNUSED outcome_type outcome {};
+ O o = make();
+ value_type v = make(S::throwing_copy_assignment);
+
+ try
+ {
+ o = v;
+ }
+ catch(exception const&)
+ { outcome = caught; }
+ catch(...)
+ { outcome = bad_catch; }
+
+ VERIFY( o && o->state == S::threw );
+ }
+
+ {
+ ATTRIBUTE_UNUSED outcome_type outcome {};
+ O o = make();
+ value_type v = make(S::throwing_move_assignment);
+
+ try
+ {
+ o = std::move(v);
+ }
+ catch(exception const&)
+ { outcome = caught; }
+ catch(...)
+ { outcome = bad_catch; }
+
+ VERIFY( o && o->state == S::threw );
+ }
+
+ VERIFY( counter == 0 );
+}
+
+} // namespace assign_4
diff --git a/gdb/unittests/optional/assignment/5.cc b/gdb/unittests/optional/assignment/5.cc
new file mode 100644
index 0000000..b1dee4f
--- /dev/null
+++ b/gdb/unittests/optional/assignment/5.cc
@@ -0,0 +1,80 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace assign_5 {
+
+int counter = 0;
+
+struct mixin_counter
+{
+ mixin_counter() { ++counter; }
+ mixin_counter(mixin_counter const&) { ++counter; }
+ ~mixin_counter() { --counter; }
+};
+
+struct value_type : private mixin_counter { };
+
+void test()
+{
+ using O = gdb::optional<value_type>;
+
+ // Check std::nullopt_t and 'default' (= {}) assignment
+
+#ifndef GDB_OPTIONAL
+ {
+ O o;
+ o = std::nullopt;
+ VERIFY( !o );
+ }
+#endif
+
+#ifndef GDB_OPTIONAL
+ {
+ O o { gdb::in_place };
+ o = std::nullopt;
+ VERIFY( !o );
+ }
+#endif
+
+#ifndef GDB_OPTIONAL
+ {
+ O o;
+ o = {};
+ VERIFY( !o );
+ }
+#endif
+
+#ifndef GDB_OPTIONAL
+ {
+ O o { gdb::in_place };
+ o = {};
+ VERIFY( !o );
+ }
+#endif
+ {
+ gdb::optional<std::vector<int>> ovi{{1, 2, 3}};
+ VERIFY(ovi->size() == 3);
+ VERIFY((*ovi)[0] == 1 && (*ovi)[1] == 2 && (*ovi)[2] == 3);
+ ovi = {4, 5, 6, 7};
+ VERIFY(ovi->size() == 4);
+ VERIFY((*ovi)[0] == 4 && (*ovi)[1] == 5 &&
+ (*ovi)[2] == 6 && (*ovi)[3] == 7);
+ }
+ VERIFY( counter == 0 );
+}
+
+} // namespace assign_5
diff --git a/gdb/unittests/optional/assignment/6.cc b/gdb/unittests/optional/assignment/6.cc
new file mode 100644
index 0000000..383ff7e
--- /dev/null
+++ b/gdb/unittests/optional/assignment/6.cc
@@ -0,0 +1,90 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace assign_6 {
+
+int counter = 0;
+
+struct mixin_counter
+{
+ mixin_counter() { ++counter; }
+ mixin_counter(mixin_counter const&) { ++counter; }
+ ~mixin_counter() { --counter; }
+};
+
+struct value_type : private mixin_counter
+{
+ value_type() = default;
+ value_type(int) : state(1) { }
+ value_type(std::initializer_list<char>, const char*) : state(2) { }
+ int state = 0;
+};
+
+void test()
+{
+ using O = gdb::optional<value_type>;
+
+ // Check emplace
+
+ {
+ O o;
+ o.emplace();
+ VERIFY( o && o->state == 0 );
+ }
+ {
+ O o { gdb::in_place, 0 };
+ o.emplace();
+ VERIFY( o && o->state == 0 );
+ }
+
+ {
+ O o;
+ o.emplace(0);
+ VERIFY( o && o->state == 1 );
+ }
+ {
+ O o { gdb::in_place };
+ o.emplace(0);
+ VERIFY( o && o->state == 1 );
+ }
+
+#ifndef GDB_OPTIONAL
+ {
+ O o;
+ o.emplace({ 'a' }, "");
+ VERIFY( o && o->state == 2 );
+ }
+ {
+ O o { gdb::in_place };
+ o.emplace({ 'a' }, "");
+ VERIFY( o && o->state == 2 );
+ }
+#endif
+ {
+ O o;
+ VERIFY(&o.emplace(0) == &*o);
+#ifndef GDB_OPTIONAL
+ VERIFY(&o.emplace({ 'a' }, "") == &*o);
+#endif
+ }
+
+ static_assert( !std::is_constructible<O, std::initializer_list<int>, int>(), "" );
+
+ VERIFY( counter == 0 );
+}
+
+} // namespace assign_6
diff --git a/gdb/unittests/optional/assignment/7.cc b/gdb/unittests/optional/assignment/7.cc
new file mode 100644
index 0000000..e23651c
--- /dev/null
+++ b/gdb/unittests/optional/assignment/7.cc
@@ -0,0 +1,29 @@
+// Copyright (C) 2016-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace assign_7 {
+
+void test()
+{
+ gdb::optional<int> o{666};
+ VERIFY(o && *o == 666);
+ o.reset();
+ VERIFY(!o);
+ static_assert(noexcept(std::declval<gdb::optional<int>>().reset()), "");
+}
+
+} // namespace assign_7
diff --git a/gdb/unittests/optional/cons/copy.cc b/gdb/unittests/optional/cons/copy.cc
new file mode 100644
index 0000000..bce423b
--- /dev/null
+++ b/gdb/unittests/optional/cons/copy.cc
@@ -0,0 +1,126 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace cons_copy {
+
+struct tracker
+{
+ tracker(int value) : value(value) { ++count; }
+ ~tracker() { --count; }
+
+ tracker(tracker const& other) : value(other.value) { ++count; }
+ tracker(tracker&& other) : value(other.value)
+ {
+ other.value = -1;
+ ++count;
+ }
+
+ tracker& operator=(tracker const&) = default;
+ tracker& operator=(tracker&&) = default;
+
+ int value;
+
+ static int count;
+};
+
+int tracker::count = 0;
+
+struct exception { };
+
+struct throwing_copy
+{
+ throwing_copy() = default;
+ throwing_copy(throwing_copy const&) { throw exception {}; }
+};
+
+void test()
+{
+ // [20.5.4.1] Constructors
+
+ {
+ gdb::optional<long> o;
+ auto copy = o;
+ VERIFY( !copy );
+ VERIFY( !o );
+ }
+
+ {
+ const long val = 0x1234ABCD;
+ gdb::optional<long> o { gdb::in_place, val};
+ auto copy = o;
+ VERIFY( copy );
+ VERIFY( *copy == val );
+#ifndef GDB_OPTIONAL
+ VERIFY( o && o == val );
+#endif
+ }
+
+ {
+ gdb::optional<tracker> o;
+ auto copy = o;
+ VERIFY( !copy );
+ VERIFY( tracker::count == 0 );
+ VERIFY( !o );
+ }
+
+ {
+ gdb::optional<tracker> o { gdb::in_place, 333 };
+ auto copy = o;
+ VERIFY( copy );
+ VERIFY( copy->value == 333 );
+ VERIFY( tracker::count == 2 );
+ VERIFY( o && o->value == 333 );
+ }
+
+ enum outcome { nothrow, caught, bad_catch };
+
+ {
+ outcome result = nothrow;
+ gdb::optional<throwing_copy> o;
+
+ try
+ {
+ auto copy = o;
+ }
+ catch(exception const&)
+ { result = caught; }
+ catch(...)
+ { result = bad_catch; }
+
+ VERIFY( result == nothrow );
+ }
+
+ {
+ outcome result = nothrow;
+ gdb::optional<throwing_copy> o { gdb::in_place };
+
+ try
+ {
+ auto copy = o;
+ }
+ catch(exception const&)
+ { result = caught; }
+ catch(...)
+ { result = bad_catch; }
+
+ VERIFY( result == caught );
+ }
+
+ VERIFY( tracker::count == 0 );
+}
+
+} // namespace cons_copy
diff --git a/gdb/unittests/optional/cons/default.cc b/gdb/unittests/optional/cons/default.cc
new file mode 100644
index 0000000..b075f5c
--- /dev/null
+++ b/gdb/unittests/optional/cons/default.cc
@@ -0,0 +1,58 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace cons_default {
+
+struct tracker
+{
+ tracker() { ++count; }
+ ~tracker() { --count; }
+
+ tracker(tracker const&) { ++count; }
+ tracker(tracker&&) { ++count; }
+
+ tracker& operator=(tracker const&) = default;
+ tracker& operator=(tracker&&) = default;
+
+ static int count;
+};
+
+int tracker::count = 0;
+
+void test()
+{
+ // [20.5.4.1] Constructors
+
+ {
+ gdb::optional<tracker> o;
+ VERIFY( !o );
+ }
+
+ {
+ gdb::optional<tracker> o {};
+ VERIFY( !o );
+ }
+
+ {
+ gdb::optional<tracker> o = {};
+ VERIFY( !o );
+ }
+
+ VERIFY( tracker::count == 0 );
+}
+
+} // namespace cons_default
diff --git a/gdb/unittests/optional/cons/move.cc b/gdb/unittests/optional/cons/move.cc
new file mode 100644
index 0000000..5a50b08
--- /dev/null
+++ b/gdb/unittests/optional/cons/move.cc
@@ -0,0 +1,124 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace cons_move {
+
+struct tracker
+{
+ tracker(int value) : value(value) { ++count; }
+ ~tracker() { --count; }
+
+ tracker(tracker const& other) : value(other.value) { ++count; }
+ tracker(tracker&& other) : value(other.value)
+ {
+ other.value = -1;
+ ++count;
+ }
+
+ tracker& operator=(tracker const&) = default;
+ tracker& operator=(tracker&&) = default;
+
+ int value;
+
+ static int count;
+};
+
+int tracker::count = 0;
+
+struct exception { };
+
+struct throwing_move
+{
+ throwing_move() = default;
+ throwing_move(throwing_move const&) { throw exception {}; }
+};
+
+void test()
+{
+ // [20.5.4.1] Constructors
+
+ {
+ gdb::optional<long> o;
+ auto moved_to = std::move(o);
+ VERIFY( !moved_to );
+ VERIFY( !o );
+ }
+
+ {
+ const long val = 0x1234ABCD;
+ gdb::optional<long> o { gdb::in_place, val};
+ auto moved_to = std::move(o);
+ VERIFY( moved_to );
+ VERIFY( *moved_to == val );
+ VERIFY( o && *o == val );
+ }
+
+ {
+ gdb::optional<tracker> o;
+ auto moved_to = std::move(o);
+ VERIFY( !moved_to );
+ VERIFY( tracker::count == 0 );
+ VERIFY( !o );
+ }
+
+ {
+ gdb::optional<tracker> o { gdb::in_place, 333 };
+ auto moved_to = std::move(o);
+ VERIFY( moved_to );
+ VERIFY( moved_to->value == 333 );
+ VERIFY( tracker::count == 2 );
+ VERIFY( o && o->value == -1 );
+ }
+
+ enum outcome { nothrow, caught, bad_catch };
+
+ {
+ outcome result = nothrow;
+ gdb::optional<throwing_move> o;
+
+ try
+ {
+ auto moved_to = std::move(o);
+ }
+ catch(exception const&)
+ { result = caught; }
+ catch(...)
+ { result = bad_catch; }
+
+ VERIFY( result == nothrow );
+ }
+
+ {
+ outcome result = nothrow;
+ gdb::optional<throwing_move> o { gdb::in_place };
+
+ try
+ {
+ auto moved_to = std::move(o);
+ }
+ catch(exception const&)
+ { result = caught; }
+ catch(...)
+ { result = bad_catch; }
+
+ VERIFY( result == caught );
+ }
+
+ VERIFY( tracker::count == 0 );
+}
+
+} // namespace cons_move
diff --git a/gdb/unittests/optional/cons/value.cc b/gdb/unittests/optional/cons/value.cc
new file mode 100644
index 0000000..52843b0
--- /dev/null
+++ b/gdb/unittests/optional/cons/value.cc
@@ -0,0 +1,294 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace cons_value {
+
+struct tracker
+{
+ tracker(int value) : value(value) { ++count; }
+ ~tracker() { --count; }
+
+ tracker(tracker const& other) : value(other.value) { ++count; }
+ tracker(tracker&& other) : value(other.value)
+ {
+ other.value = -1;
+ ++count;
+ }
+
+ tracker& operator=(tracker const&) = default;
+ tracker& operator=(tracker&&) = default;
+
+ int value;
+
+ static int count;
+};
+
+int tracker::count = 0;
+
+struct exception { };
+
+struct throwing_construction
+{
+ explicit throwing_construction(bool propagate) : propagate(propagate) { }
+
+ throwing_construction(throwing_construction const& other)
+ : propagate(other.propagate)
+ {
+ if(propagate)
+ throw exception {};
+ }
+
+ bool propagate;
+};
+
+void test()
+{
+ // [20.5.4.1] Constructors
+
+ {
+ auto i = 0x1234ABCD;
+ gdb::optional<long> o { i };
+ VERIFY( o );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
+ }
+
+ {
+ auto i = 0x1234ABCD;
+ gdb::optional<long> o = i;
+ VERIFY( o );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
+ }
+
+ {
+ auto i = 0x1234ABCD;
+ gdb::optional<long> o = { i };
+ VERIFY( o );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
+ }
+
+ {
+ auto i = 0x1234ABCD;
+ gdb::optional<long> o { std::move(i) };
+ VERIFY( o );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
+ }
+
+ {
+ auto i = 0x1234ABCD;
+ gdb::optional<long> o = std::move(i);
+ VERIFY( o );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
+ }
+
+ {
+ auto i = 0x1234ABCD;
+ gdb::optional<long> o = { std::move(i) };
+ VERIFY( o );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
+ }
+
+#ifndef GDB_OPTIONAL
+ {
+ std::vector<int> v = { 0, 1, 2, 3, 4, 5 };
+ gdb::optional<std::vector<int>> o { v };
+ VERIFY( !v.empty() );
+ VERIFY( o->size() == 6 );
+ }
+#endif
+
+ {
+ std::vector<int> v = { 0, 1, 2, 3, 4, 5 };
+ gdb::optional<std::vector<int>> o = v;
+ VERIFY( !v.empty() );
+ VERIFY( o->size() == 6 );
+ }
+
+ {
+ std::vector<int> v = { 0, 1, 2, 3, 4, 5 };
+ gdb::optional<std::vector<int>> o { v };
+ VERIFY( !v.empty() );
+ VERIFY( o->size() == 6 );
+ }
+
+ {
+ std::vector<int> v = { 0, 1, 2, 3, 4, 5 };
+ gdb::optional<std::vector<int>> o { std::move(v) };
+ VERIFY( v.empty() );
+ VERIFY( o->size() == 6 );
+ }
+
+ {
+ std::vector<int> v = { 0, 1, 2, 3, 4, 5 };
+ gdb::optional<std::vector<int>> o = std::move(v);
+ VERIFY( v.empty() );
+ VERIFY( o->size() == 6 );
+ }
+
+ {
+ std::vector<int> v = { 0, 1, 2, 3, 4, 5 };
+ gdb::optional<std::vector<int>> o { std::move(v) };
+ VERIFY( v.empty() );
+ VERIFY( o->size() == 6 );
+ }
+
+ {
+ tracker t { 333 };
+ gdb::optional<tracker> o = t;
+ VERIFY( o->value == 333 );
+ VERIFY( tracker::count == 2 );
+ VERIFY( t.value == 333 );
+ }
+
+ {
+ tracker t { 333 };
+ gdb::optional<tracker> o = std::move(t);
+ VERIFY( o->value == 333 );
+ VERIFY( tracker::count == 2 );
+ VERIFY( t.value == -1 );
+ }
+
+ enum outcome { nothrow, caught, bad_catch };
+
+ {
+ outcome result = nothrow;
+ throwing_construction t { false };
+
+ try
+ {
+ gdb::optional<throwing_construction> o { t };
+ }
+ catch(exception const&)
+ { result = caught; }
+ catch(...)
+ { result = bad_catch; }
+
+ VERIFY( result == nothrow );
+ }
+
+ {
+ outcome result = nothrow;
+ throwing_construction t { true };
+
+ try
+ {
+ gdb::optional<throwing_construction> o { t };
+ }
+ catch(exception const&)
+ { result = caught; }
+ catch(...)
+ { result = bad_catch; }
+
+ VERIFY( result == caught );
+ }
+
+ {
+ outcome result = nothrow;
+ throwing_construction t { false };
+
+ try
+ {
+ gdb::optional<throwing_construction> o { std::move(t) };
+ }
+ catch(exception const&)
+ { result = caught; }
+ catch(...)
+ { result = bad_catch; }
+
+ VERIFY( result == nothrow );
+ }
+
+ {
+ outcome result = nothrow;
+ throwing_construction t { true };
+
+ try
+ {
+ gdb::optional<throwing_construction> o { std::move(t) };
+ }
+ catch(exception const&)
+ { result = caught; }
+ catch(...)
+ { result = bad_catch; }
+
+ VERIFY( result == caught );
+ }
+
+ {
+#ifndef GDB_OPTIONAL
+ gdb::optional<std::string> os = "foo";
+#endif
+ struct X
+ {
+ explicit X(int) {}
+ X& operator=(int) {return *this;}
+ };
+#ifndef GDB_OPTIONAL
+ gdb::optional<X> ox{42};
+#endif
+ gdb::optional<int> oi{42};
+#ifndef GDB_OPTIONAL
+ gdb::optional<X> ox2{oi};
+#endif
+ gdb::optional<std::string> os2;
+ os2 = "foo";
+#ifndef GDB_OPTIONAL
+ gdb::optional<X> ox3;
+ ox3 = 42;
+ gdb::optional<X> ox4;
+ ox4 = oi;
+#endif
+ }
+ {
+ // no converting construction.
+#ifndef GDB_OPTIONAL
+ gdb::optional<int> oi = gdb::optional<short>();
+ VERIFY(!bool(oi));
+ gdb::optional<std::string> os = gdb::optional<const char*>();
+ VERIFY(!bool(os));
+#endif
+ gdb::optional<gdb::optional<int>> ooi = gdb::optional<int>();
+ VERIFY(bool(ooi));
+ ooi = gdb::optional<int>();
+ VERIFY(bool(ooi));
+ ooi = gdb::optional<int>(42);
+ VERIFY(bool(ooi));
+ VERIFY(bool(*ooi));
+#ifndef GDB_OPTIONAL
+ gdb::optional<gdb::optional<int>> ooi2 = gdb::optional<short>();
+ VERIFY(bool(ooi2));
+ ooi2 = gdb::optional<short>();
+ VERIFY(bool(ooi2));
+ ooi2 = gdb::optional<short>(6);
+ VERIFY(bool(ooi2));
+ VERIFY(bool(*ooi2));
+ gdb::optional<gdb::optional<int>> ooi3 = gdb::optional<int>(42);
+ VERIFY(bool(ooi3));
+ VERIFY(bool(*ooi3));
+ gdb::optional<gdb::optional<int>> ooi4 = gdb::optional<short>(6);
+ VERIFY(bool(ooi4));
+ VERIFY(bool(*ooi4));
+#endif
+ }
+}
+
+} // namespace cons_value
diff --git a/gdb/unittests/optional/in_place.cc b/gdb/unittests/optional/in_place.cc
new file mode 100644
index 0000000..5ddaece
--- /dev/null
+++ b/gdb/unittests/optional/in_place.cc
@@ -0,0 +1,65 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace in_place {
+
+void test()
+{
+ // [20.5.5] In-place construction
+ {
+ gdb::optional<int> o { gdb::in_place };
+ VERIFY( o );
+ VERIFY( *o == int() );
+
+#ifndef GDB_OPTIONAL
+ static_assert( !std::is_convertible<gdb::in_place_t, gdb::optional<int>>(), "" );
+#endif
+ }
+
+ {
+ gdb::optional<int> o { gdb::in_place, 42 };
+ VERIFY( o );
+ VERIFY( *o == 42 );
+ }
+
+ {
+ gdb::optional<std::vector<int>> o { gdb::in_place, 18, 4 };
+ VERIFY( o );
+ VERIFY( o->size() == 18 );
+ VERIFY( (*o)[17] == 4 );
+ }
+
+#ifndef GDB_OPTIONAL
+ {
+ gdb::optional<std::vector<int>> o { gdb::in_place, { 18, 4 } };
+ VERIFY( o );
+ VERIFY( o->size() == 2 );
+ VERIFY( (*o)[0] == 18 );
+ }
+#endif
+
+#ifndef GDB_OPTIONAL
+ {
+ gdb::optional<std::vector<int>> o { gdb::in_place, { 18, 4 }, std::allocator<int> {} };
+ VERIFY( o );
+ VERIFY( o->size() == 2 );
+ VERIFY( (*o)[0] == 18 );
+ }
+#endif
+}
+
+} // namespace in_place
diff --git a/gdb/unittests/optional/observers/1.cc b/gdb/unittests/optional/observers/1.cc
new file mode 100644
index 0000000..6c8677e
--- /dev/null
+++ b/gdb/unittests/optional/observers/1.cc
@@ -0,0 +1,31 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace observers_1 {
+
+struct value_type
+{
+ int i;
+};
+
+void test()
+{
+ gdb::optional<value_type> o { value_type { 51 } };
+ VERIFY( (*o).i == 51 );
+}
+
+} // namespace observers_1
diff --git a/gdb/unittests/optional/observers/2.cc b/gdb/unittests/optional/observers/2.cc
new file mode 100644
index 0000000..3af49e6
--- /dev/null
+++ b/gdb/unittests/optional/observers/2.cc
@@ -0,0 +1,35 @@
+// Copyright (C) 2013-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+namespace observers_2 {
+
+struct value_type
+{
+ int i;
+};
+
+void* operator&(const value_type&) = delete;
+
+void test()
+{
+ gdb::optional<value_type> o { value_type { 51 } };
+ VERIFY( o->i == 51 );
+ VERIFY( o->i == (*o).i );
+ VERIFY( &o->i == &(*o).i );
+}
+
+} // namespace observers_2
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/7] C++fy xml-support.c and more gdb::optional
2017-04-10 12:22 [PATCH 0/7] C++fy xml-support.c and more gdb::optional Pedro Alves
` (6 preceding siblings ...)
2017-04-10 12:32 ` [PATCH 4/7] gdb::optional unit tests Pedro Alves
@ 2017-04-15 16:20 ` Simon Marchi
2017-04-18 22:58 ` Pedro Alves
7 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2017-04-15 16:20 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 2017-04-10 08:22, Pedro Alves wrote:
> I've been sitting on this series for a while now as part of the whole
> "eliminate cleanups" effort, and since other folks are looking at
> eliminating cleanups too, I thought I'd get this one over with to
> avoid duplication.
>
> In one of the patches (and also in [1] and in another larger series
> I'm working on) I kept wanting to use gdb::optional, but finding
> "opt.emplace(....)" a bit cumbersome compared to the more natural
> ctor/op=. So I bit the bullet and brought in a bit more of C++17's
> std::optional API into gdb::optional.
>
> I then tried to follow the "Unit tests or it didn't happen (TM)"
> principle and added unit tests for gdb::optional. That payed off and
> caught some bugs in the new code I was adding.
>
> [1] - https://sourceware.org/ml/gdb-patches/2017-03/msg00497.html
>
> Tested on x86-64 Fedora 23, native and gdbserver.
I looked at the series at a high level, it looks good. I don't mind
about the tests, they can only be helpful.
I am actually happy about the change to optional, because I recently
tried to do a function that returned an optional and found out it wasn't
possible yet.
Thanks,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/7] C++fy xml-support.c and more gdb::optional
2017-04-15 16:20 ` [PATCH 0/7] C++fy xml-support.c and more gdb::optional Simon Marchi
@ 2017-04-18 22:58 ` Pedro Alves
0 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2017-04-18 22:58 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On 04/15/2017 05:20 PM, Simon Marchi wrote:
> On 2017-04-10 08:22, Pedro Alves wrote:
>> I've been sitting on this series for a while now as part of the whole
>> "eliminate cleanups" effort, and since other folks are looking at
>> eliminating cleanups too, I thought I'd get this one over with to
>> avoid duplication.
>>
>> In one of the patches (and also in [1] and in another larger series
>> I'm working on) I kept wanting to use gdb::optional, but finding
>> "opt.emplace(....)" a bit cumbersome compared to the more natural
>> ctor/op=. So I bit the bullet and brought in a bit more of C++17's
>> std::optional API into gdb::optional.
>>
>> I then tried to follow the "Unit tests or it didn't happen (TM)"
>> principle and added unit tests for gdb::optional. That payed off and
>> caught some bugs in the new code I was adding.
>>
>> [1] - https://sourceware.org/ml/gdb-patches/2017-03/msg00497.html
>>
>> Tested on x86-64 Fedora 23, native and gdbserver.
>
> I looked at the series at a high level, it looks good. I don't mind
> about the tests, they can only be helpful.
Thanks, I've pushed it in now.
> I am actually happy about the change to optional, because I recently
> tried to do a function that returned an optional and found out it wasn't
> possible yet.
Oh, that's right. I should have mentioned that in the commit log.
I've done that now. I'll reply to that patch with with I pushed.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 13+ messages in thread