Attached is a patch for p1301 that improves in the way Jason Merrill specified earlier (https://gcc.gnu.org/ml/gcc-patches/2019-09/msg00858.html), but it keeps segfaulting on my build of GCC. I don't know what changes I've made that cause it to segfault: it does so whenever the error() function is called but the backtraces aren't showing me anything conclusive.
The tests test what I expect them to and the output is fine, I just can't get the segfaults to stop, so I'm putting the patch up so someone can critique what I've written, or someone else to test it too. Sorry for taking so long. Thanks, JeanHeyd
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c index e3c602fbb8d..fb05b5f8af0 100644 --- a/gcc/c-family/c-lex.c +++ b/gcc/c-family/c-lex.c @@ -353,13 +353,14 @@ c_common_has_attribute (cpp_reader *pfile) else if (is_attribute_p ("deprecated", attr_name)) result = 201309; else if (is_attribute_p ("maybe_unused", attr_name) - || is_attribute_p ("nodiscard", attr_name) || is_attribute_p ("fallthrough", attr_name)) result = 201603; else if (is_attribute_p ("no_unique_address", attr_name) || is_attribute_p ("likely", attr_name) || is_attribute_p ("unlikely", attr_name)) result = 201803; + else if (is_attribute_p ("nodiscard", attr_name)) + result = 201907; if (result) attr_name = NULL_TREE; } diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c index 364af72e68d..3365219636f 100644 --- a/gcc/cp/cvt.c +++ b/gcc/cp/cvt.c @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see #include "convert.h" #include "stringpool.h" #include "attribs.h" +#include "escaped_string.h" static tree convert_to_pointer_force (tree, tree, tsubst_flags_t); static tree build_type_conversion (tree, tree); @@ -1026,22 +1027,45 @@ maybe_warn_nodiscard (tree expr, impl_conv_void implicit) tree rettype = TREE_TYPE (type); tree fn = cp_get_fndecl_from_callee (callee); + tree attr; if (implicit != ICV_CAST && fn - && lookup_attribute ("nodiscard", DECL_ATTRIBUTES (fn))) + && (attr = lookup_attribute ("nodiscard", DECL_ATTRIBUTES (fn)))) { + escaped_string msg; + tree args = TREE_VALUE(attr); + const bool has_string_arg = args && TREE_CODE (TREE_VALUE (args)) == STRING_CST; + if (has_string_arg) + msg.escape (TREE_STRING_POINTER (TREE_VALUE (args))); + const bool has_msg = msg; + const char* format = (has_msg ? + G_("ignoring return value of %qD, " + "declared with attribute %<nodiscard%>: %<%s%>") : + G_("ignoring return value of %qD, " + "declared with attribute %<nodiscard%>%s")); + const char* raw_msg = (has_msg ? static_cast<const char*>(msg) : ""); auto_diagnostic_group d; if (warning_at (loc, OPT_Wunused_result, - "ignoring return value of %qD, " - "declared with attribute nodiscard", fn)) + format, fn, raw_msg)) inform (DECL_SOURCE_LOCATION (fn), "declared here"); } else if (implicit != ICV_CAST - && lookup_attribute ("nodiscard", TYPE_ATTRIBUTES (rettype))) + && (attr = lookup_attribute ("nodiscard", TYPE_ATTRIBUTES (rettype)))) { + escaped_string msg; + tree args = TREE_VALUE(attr); + const bool has_string_arg = args && TREE_CODE (TREE_VALUE (args)) == STRING_CST; + if (has_string_arg) + msg.escape (TREE_STRING_POINTER (TREE_VALUE (args))); + const bool has_msg = msg; + const char* format = has_msg ? + G_("ignoring return value of type %qT, " + "declared with attribute %<nodiscard%>: %<%s%>") : + G_("ignoring return value of type %qT, " + "declared with attribute %<nodiscard%>%s"); + const char* raw_msg = (has_msg ? static_cast<const char*>(msg) : ""); auto_diagnostic_group d; if (warning_at (loc, OPT_Wunused_result, - "ignoring returned value of type %qT, " - "declared with attribute nodiscard", rettype)) + format, rettype, raw_msg)) { if (fn) inform (DECL_SOURCE_LOCATION (fn), @@ -1050,24 +1074,59 @@ maybe_warn_nodiscard (tree expr, impl_conv_void implicit) "%qT declared here", rettype); } } - else if (TREE_CODE (expr) == TARGET_EXPR - && lookup_attribute ("warn_unused_result", TYPE_ATTRIBUTES (type))) + else if (TREE_CODE (expr) == TARGET_EXPR) { /* The TARGET_EXPR confuses do_warn_unused_result into thinking that the result is used, so handle that case here. */ - if (fn) + if (lookup_attribute ("warn_unused_result", TYPE_ATTRIBUTES (type))) { - auto_diagnostic_group d; - if (warning_at (loc, OPT_Wunused_result, - "ignoring return value of %qD, " - "declared with attribute %<warn_unused_result%>", - fn)) - inform (DECL_SOURCE_LOCATION (fn), "declared here"); + if (fn) + { + auto_diagnostic_group d; + if (warning_at (loc, OPT_Wunused_result, + "ignoring return value of %qD, " + "declared with attribute %<warn_unused_result%>", + fn)) + inform (DECL_SOURCE_LOCATION (fn), "declared here"); + } + else + warning_at (loc, OPT_Wunused_result, + "ignoring return value of function " + "declared with attribute %<warn_unused_result%>"); + } + else if ((attr = lookup_attribute ("nodiscard", TYPE_ATTRIBUTES (type)))) + { + escaped_string msg; + tree args = TREE_VALUE(attr); + const bool has_string_arg = args && TREE_CODE (TREE_VALUE (args)) == STRING_CST; + if (has_string_arg) + msg.escape (TREE_STRING_POINTER (TREE_VALUE (args))); + if (fn) + { + const bool has_msg = msg; + const char* format = has_msg + ? G_("ignoring return value of %qD, " + "declared with attribute %<nodiscard%>: %<%s%>") + : G_("ignoring return value of %qD, " + "declared with attribute %<nodiscard%>%s"); + const char* raw_msg = (has_msg ? static_cast<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"); + } + else + { + const bool has_msg = msg; + const char* format = has_msg + ? G_("ignoring return value of function " + "declared with attribute %<nodiscard%>: %<%s%>") + : G_("ignoring return value of function " + "declared with attribute %<nodiscard%>%s"); + const char* raw_msg = (has_msg ? static_cast<const char*>(msg) : ""); + warning_at (loc, OPT_Wunused_result, format, raw_msg); + } } - else - warning_at (loc, OPT_Wunused_result, - "ignoring return value of function " - "declared with attribute %<warn_unused_result%>"); } } @@ -1180,7 +1239,7 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain) instantiations be affected by an ABI property that is, or at least ought to be transparent to the language. */ if (tree fn = cp_get_callee_fndecl_nofold (expr)) - if (DECL_CONSTRUCTOR_P (fn) || DECL_DESTRUCTOR_P (fn)) + if (DECL_DESTRUCTOR_P (fn)) return expr; maybe_warn_nodiscard (expr, implicit); diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 53db8afc9f6..2ace912bfa3 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -26568,10 +26568,10 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) return attribute; } -/* Check that the attribute ATTRIBUTE appears at most once in the - attribute-list ATTRIBUTES. This is enforced for noreturn (7.6.3) - and deprecated (7.6.5). Note that carries_dependency (7.6.4) - isn't implemented yet in GCC. */ +/* Check that the attribute ATTRIBU,TE appears at most once in the + attribute-list ATTRIBUTES. This is enforced for noreturn (7.6.3), + nodiscard, and deprecated (7.6.5). Note that + carries_dependency (7.6.4) isn't implemented yet in GCC. */ static void cp_parser_check_std_attribute (tree attributes, tree attribute) @@ -26587,6 +26587,10 @@ cp_parser_check_std_attribute (tree attributes, tree attribute) && lookup_attribute ("deprecated", attributes)) error ("attribute %<deprecated%> can appear at most once " "in an attribute-list"); + else if (is_attribute_p ("nodiscard", name) + && lookup_attribute ("nodiscard", attributes)) + error ("attribute %<nodiscard%> can appear at most once " + "in an attribute-list"); } } diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index 7f71891000d..a004bb1aa7f 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -4369,9 +4369,14 @@ zero_init_p (const_tree t) warn_unused_result attribute. */ static tree -handle_nodiscard_attribute (tree *node, tree name, tree /*args*/, +handle_nodiscard_attribute (tree *node, tree name, tree args, int /*flags*/, bool *no_add_attrs) { + if (args && TREE_CODE (TREE_VALUE (args)) != STRING_CST) + { + error ("%qE attribute argument must be a string constant", name); + *no_add_attrs = true; + } if (TREE_CODE (*node) == FUNCTION_DECL) { if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (*node))) @@ -4461,7 +4466,7 @@ const struct attribute_spec std_attribute_table[] = affects_type_identity, handler, exclude } */ { "maybe_unused", 0, 0, false, false, false, false, handle_unused_attribute, NULL }, - { "nodiscard", 0, 0, false, false, false, false, + { "nodiscard", 0, 1, false, false, false, false, handle_nodiscard_attribute, NULL }, { "no_unique_address", 0, 0, true, false, false, false, handle_no_unique_addr_attribute, NULL }, diff --git a/gcc/escaped_string.h b/gcc/escaped_string.h new file mode 100644 index 00000000000..07dca4cf53e --- /dev/null +++ b/gcc/escaped_string.h @@ -0,0 +1,43 @@ +/* Shared escaped string class. + Copyright (C) 1999-2019 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC 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. + +GCC 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 GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +#ifndef GCC_ESCAPED_STRING_H +#define GCC_ESCAPED_STRING_H + +#include <cstdlib> + +/* A class to handle converting a string that might contain + control characters, (eg newline, form-feed, etc), into one + in which contains escape sequences instead. */ + +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 m_str; } + void escape (const char *); + private: + escaped_string(const escaped_string&) {} + escaped_string& operator=(const escaped_string&) { return *this; } + char *m_str; + bool m_owned; +}; + +#endif /* ! GCC_ESCAPED_STRING_H */ diff --git a/gcc/tree.c b/gcc/tree.c index e845fc7a00e..2bee1d255ff 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -68,6 +68,7 @@ along with GCC; see the file COPYING3. If not see #include "regs.h" #include "tree-vector-builder.h" #include "gimple-fold.h" +#include "escaped_string.h" /* Tree code classes. */ @@ -13225,22 +13226,6 @@ typedef_variant_p (const_tree type) return is_typedef_decl (TYPE_NAME (type)); } -/* A class to handle converting a string that might contain - control characters, (eg newline, form-feed, etc), into one - in which contains escape sequences instead. */ - -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; } - void escape (const char *); - private: - char *m_str; - bool m_owned; -}; - /* PR 84195: Replace control characters in "unescaped" with their escaped equivalents. Allow newlines if -fmessage-length has been set to a non-zero value. This is done here, rather than