Hi, thanks a lot for the patch, and I'm sorry I didn't notice it sooner;
I've adjusted my mail filters to hopefully avoid similar problems. In
the future, please CC me on C++ patches and put "C++" and "PATCH" in the
subject line.
On 7/25/19 7:57 PM, phdoftheho...@gmail.com wrote:
From: ThePhD <phdoftheho...@gmail.com>
07-24-2019 ThePhD <phdoftheho...@gmail.com>
Please use your real name in the git Author field and ChangeLog entries.
There are other PhDs around :)
In order to get the ChangeLog into the initial comment in git
send-patch, I add it to the git commit message.
Attaching a patch to your email is also fine for gcc-patches.
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index 851fd704e5d..f5870d095f2 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -345,24 +345,26 @@ c_common_has_attribute (cpp_reader *pfile)
attr_name = NULL_TREE;
}
}
- else
- {
- /* Some standard attributes need special handling. */
+ else
+ {
+ /* Some standard attributes need special
handling. */
Please avoid changing whitespace on existing lines. In this case you've
reindented existing lines to no longer conform to the GNU coding
standards, which use 2-space indents; this happens throughout your patch.
+ else if (is_attribute_p ("nodiscard",
attr_name))
+ result = 201907; /* placeholder until
C++20 Post-Cologne Working Draft. */
The comment can be removed.
diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index 23d2aabc483..4a5128c76a1 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
+ if (implicit != ICV_CAST && fn
+ && lookup_attribute ("nodiscard", DECL_ATTRIBUTES (fn)))
+ {
+ tree attr = DECL_ATTRIBUTES (fn);
This will break if the function has other attributes. Instead, set attr
to the result of lookup_attribute.
+ bool has_msg = static_cast<bool>(msg);
The static_cast seems unnecessary; you're already requiring a conversion
to bool for the variable initialization.
+ const char* format = (has_msg ?
+ "ignoring return value of %qD, "
+ "declared with attribute %<nodiscard%>: %<%s%>"
:
+ "ignoring return value of %qD, "
+ "declared with attribute %<nodiscard%>%s");
Error messages that aren't passed directly to a function like "error" or
"warning" must be escaped with the G_() macro so that they can be
translated.
+ const char* raw_msg = (has_msg ? (const char*)msg : "");
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wunused_result,
+ format, fn, raw_msg))
+ {
+ inform (DECL_SOURCE_LOCATION (fn), "declared
here");
+ }
We usually don't wrap a single statement in braces. Please don't add
them around existing code unless you're adding more statements.
+++ b/gcc/cp/parser.c
@@ -26399,13 +26399,26 @@ cp_parser_check_std_attribute (tree attributes, tree
attribute)
+ else if (cxx_dialect >= cxx2a)
+ {
+ if (is_attribute_p ("nodiscard", name)
+ && lookup_attribute ("nodiscard", attributes))
+ {
+ error ("%attribute qE can appear at most once "
+ "in an attribute-list", name);
+ }
This check shouldn't be limited to C++2a.
+++ b/gcc/cp/tree.c
+handle_nodiscard_attribute (tree *node, tree name, tree args,
int /*flags*/, bool *no_add_attrs)
{
+ if (!args)
+ *no_add_attrs = true;
This makes GCC ignore the nodiscard attribute if there are no arguments;
we don't want to set *no_add_attrs here.
+ else if (cxx_dialect >= cxx2a)
...
+ else
+ {
+ if (!*no_add_attrs)
+ {
+ error ("%qE attribute with an argument only
available with "
+ "%<-std=c++2a%> or
%<-std=gnu++2a%>", name);
+ }
+ }
The standard says, "For an attribute-token (including an
attribute-scoped-token) not specified in this document, the behavior is
implementation-defined. Any attribute-token that is not recognized by
the implementation is ignored."
So this error is non-conforming. Generally we just support the
attribute in all standard modes.
I also don't know why you are checking *no_add_attrs here, nothing can
set it earlier in the function.
+class escaped_string
+{
+ public:
+ escaped_string () { m_owned = false; m_str = NULL; };
+ ~escaped_string () { if (m_owned) free (m_str); }
+ operator const char *() const { return (const char *) m_str; }
As long as we're moving this class around, let's remove this unnecessary
cast.
Jason
