From: Jason Merrill <jason@redhat.com>
To: David Malcolm <dmalcolm@redhat.com>
Cc: gcc-patches List <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 4/4] C++: more std header hints; filter on C++ dialect (PR c++/84269)
Date: Thu, 29 Mar 2018 19:30:00 -0000 [thread overview]
Message-ID: <CADzB+2kQ60V4_sAxYAShDkJBYaC-q-f1NA3NuMXmNkfsNXBFEw@mail.gmail.com> (raw)
In-Reply-To: <1521762258-50849-5-git-send-email-dmalcolm@redhat.com>
OK.
On Thu, Mar 22, 2018 at 7:44 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> Jonathan added a bunch more suggestions to:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84269#c10
> as I was testing my last patch, some of which need C++14 and C++17,
> and some of which use headers that exist in earlier standards.
>
> For example, <memory> exists in C++98, but if the user attempts to
> use std::make_shared with -std=c++98, they are suggested to include
> <memory>, even if they've already included it.
>
> This patch adds the missing names, and fixes the nonsensical suggestions
> by detecting if the name isn't available yet, based on the user's
> dialect, and reporting things more intelligently:
>
> t.cc: In function 'void test_make_shared()':
> t.cc:5:8: error: 'make_shared' is not a member of 'std'
> std::make_shared<int>();
> ^~~~~~~~~~~
> t.cc:5:8: note: 'std::make_shared' is only available from C++11 onwards
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> PR c++/84269
> * name-lookup.c (struct std_name_hint): Move out of
> get_std_name_hint; add field "min_dialect". */
> (get_std_name_hint): Add min_dialect values to all initializers.
> Add <any>, <atomic>, <bitset>, <condition_variable>, <functional>,
> <future>, <istream>, <iterator>, <ostream>, <mutex>, <optional>,
> <shared_mutex>, <string_view>, <thread>, and <variant>.
> Add fstream, ifstream, and ofstream to <fstream>.
> Add istringstream, ostringstream, and stringstream to <sstream>.
> Add basic_string to <string>.
> Add tuple_element and tuple_size to <tuple>.
> Add declval to <utility>.
> Fix ordering of <queue> and <tuple>.
> Return a std_name_hint, rather than a const char *.
> (get_cxx_dialect_name): New function.
> (maybe_suggest_missing_std_header): Detect names that aren't yet
> available in the current dialect, and instead of suggesting a
> missing #include, warn about the dialect.
>
> gcc/testsuite/ChangeLog:
> PR c++/84269
> * g++.dg/lookup/missing-std-include-6.C: Move std::array and
> std::tuple here since they need C++11.
> * g++.dg/lookup/missing-std-include-8.C: New test.
> * g++.dg/lookup/missing-std-include.C: Move std::array and
> std::tuple test to missing-std-include-6.C to avoid failures
> with C++98.
> ---
> gcc/cp/name-lookup.c | 262 +++++++++++++++------
> .../g++.dg/lookup/missing-std-include-6.C | 13 +
> .../g++.dg/lookup/missing-std-include-8.C | 44 ++++
> gcc/testsuite/g++.dg/lookup/missing-std-include.C | 7 -
> 4 files changed, 248 insertions(+), 78 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 4eb980e..3d676bb 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -5441,104 +5441,214 @@ suggest_alternatives_for (location_t location, tree name,
> }
> }
>
> +/* A well-known name within the C++ standard library, returned by
> + get_std_name_hint. */
> +
> +struct std_name_hint
> +{
> + /* A name within "std::". */
> + const char *name;
> +
> + /* The header name defining it within the C++ Standard Library
> + (with '<' and '>'). */
> + const char *header;
> +
> + /* The dialect of C++ in which this was added. */
> + enum cxx_dialect min_dialect;
> +};
> +
> /* Subroutine of maybe_suggest_missing_header for handling unrecognized names
> for some of the most common names within "std::".
> - Given non-NULL NAME, a name for lookup within "std::", return the header
> - name defining it within the C++ Standard Library (with '<' and '>'),
> - or NULL. */
> + Given non-NULL NAME, return the std_name_hint for it, or NULL. */
>
> -static const char *
> +static const std_name_hint *
> get_std_name_hint (const char *name)
> {
> - struct std_name_hint
> - {
> - const char *name;
> - const char *header;
> - };
> static const std_name_hint hints[] = {
> + /* <any>. */
> + {"any", "<any>", cxx17},
> + {"any_cast", "<any>", cxx17},
> + {"make_any", "<any>", cxx17},
> /* <array>. */
> - {"array", "<array>"}, // C++11
> + {"array", "<array>", cxx11},
> + /* <atomic>. */
> + {"atomic", "<atomic>", cxx11},
> + {"atomic_flag", "<atomic>", cxx11},
> + /* <bitset>. */
> + {"bitset", "<bitset>", cxx11},
> /* <complex>. */
> - {"complex", "<complex>"},
> - {"complex_literals", "<complex>"},
> + {"complex", "<complex>", cxx98},
> + {"complex_literals", "<complex>", cxx98},
> + /* <condition_variable>. */
> + {"condition_variable", "<condition_variable>", cxx11},
> + {"condition_variable_any", "<condition_variable>", cxx11},
> /* <deque>. */
> - {"deque", "<deque>"},
> + {"deque", "<deque>", cxx98},
> /* <forward_list>. */
> - {"forward_list", "<forward_list>"}, // C++11
> + {"forward_list", "<forward_list>", cxx11},
> /* <fstream>. */
> - {"basic_filebuf", "<fstream>"},
> - {"basic_ifstream", "<fstream>"},
> - {"basic_ofstream", "<fstream>"},
> - {"basic_fstream", "<fstream>"},
> + {"basic_filebuf", "<fstream>", cxx98},
> + {"basic_ifstream", "<fstream>", cxx98},
> + {"basic_ofstream", "<fstream>", cxx98},
> + {"basic_fstream", "<fstream>", cxx98},
> + {"fstream", "<fstream>", cxx98},
> + {"ifstream", "<fstream>", cxx98},
> + {"ofstream", "<fstream>", cxx98},
> + /* <functional>. */
> + {"bind", "<functional>", cxx11},
> + {"function", "<functional>", cxx11},
> + {"hash", "<functional>", cxx11},
> + {"mem_fn", "<functional>", cxx11},
> + /* <future>. */
> + {"async", "<future>", cxx11},
> + {"future", "<future>", cxx11},
> + {"packaged_task", "<future>", cxx11},
> + {"promise", "<future>", cxx11},
> /* <iostream>. */
> - {"cin", "<iostream>"},
> - {"cout", "<iostream>"},
> - {"cerr", "<iostream>"},
> - {"clog", "<iostream>"},
> - {"wcin", "<iostream>"},
> - {"wcout", "<iostream>"},
> - {"wclog", "<iostream>"},
> + {"cin", "<iostream>", cxx98},
> + {"cout", "<iostream>", cxx98},
> + {"cerr", "<iostream>", cxx98},
> + {"clog", "<iostream>", cxx98},
> + {"wcin", "<iostream>", cxx98},
> + {"wcout", "<iostream>", cxx98},
> + {"wclog", "<iostream>", cxx98},
> + /* <istream>. */
> + {"istream", "<istream>", cxx98},
> + /* <iterator>. */
> + {"advance", "<iterator>", cxx98},
> + {"back_inserter", "<iterator>", cxx98},
> + {"begin", "<iterator>", cxx11},
> + {"distance", "<iterator>", cxx98},
> + {"end", "<iterator>", cxx11},
> + {"front_inserter", "<iterator>", cxx98},
> + {"inserter", "<iterator>", cxx98},
> + {"istream_iterator", "<iterator>", cxx98},
> + {"istreambuf_iterator", "<iterator>", cxx98},
> + {"iterator_traits", "<iterator>", cxx98},
> + {"move_iterator", "<iterator>", cxx11},
> + {"next", "<iterator>", cxx11},
> + {"ostream_iterator", "<iterator>", cxx98},
> + {"ostreambuf_iterator", "<iterator>", cxx98},
> + {"prev", "<iterator>", cxx11},
> + {"reverse_iterator", "<iterator>", cxx98},
> + /* <ostream>. */
> + {"ostream", "<ostream>", cxx98},
> /* <list>. */
> - {"list", "<list>"},
> + {"list", "<list>", cxx98},
> /* <map>. */
> - {"map", "<map>"},
> - {"multimap", "<map>"},
> + {"map", "<map>", cxx98},
> + {"multimap", "<map>", cxx98},
> /* <memory>. */
> - {"make_shared", "<memory>"},
> - {"make_unique", "<memory>"},
> - {"shared_ptr", "<memory>"},
> - {"unique_ptr", "<memory>"},
> - {"weak_ptr", "<memory>"},
> - /* <queue>. */
> - {"queue", "<queue>"},
> - {"priority_queue", "<queue>"},
> + {"make_shared", "<memory>", cxx11},
> + {"make_unique", "<memory>", cxx11},
> + {"shared_ptr", "<memory>", cxx11},
> + {"unique_ptr", "<memory>", cxx11},
> + {"weak_ptr", "<memory>", cxx11},
> + /* <mutex>. */
> + {"mutex", "<mutex>", cxx11},
> + {"timed_mutex", "<mutex>", cxx11},
> + {"recursive_mutex", "<mutex>", cxx11},
> + {"recursive_timed_mutex", "<mutex>", cxx11},
> + {"once_flag", "<mutex>", cxx11},
> + {"call_once,", "<mutex>", cxx11},
> + {"lock", "<mutex>", cxx11},
> + {"scoped_lock", "<mutex>", cxx17},
> + {"try_lock", "<mutex>", cxx11},
> + {"lock_guard", "<mutex>", cxx11},
> + {"unique_lock", "<mutex>", cxx11},
> + /* <optional>. */
> + {"optional", "<optional>", cxx17},
> + {"make_optional", "<optional>", cxx17},
> /* <ostream>. */
> - {"ostream", "<ostream>"},
> - {"wostream", "<ostream>"},
> - {"ends", "<ostream>"},
> - {"flush", "<ostream>"},
> - {"endl", "<ostream>"},
> + {"ostream", "<ostream>", cxx98},
> + {"wostream", "<ostream>", cxx98},
> + {"ends", "<ostream>", cxx98},
> + {"flush", "<ostream>", cxx98},
> + {"endl", "<ostream>", cxx98},
> + /* <queue>. */
> + {"queue", "<queue>", cxx98},
> + {"priority_queue", "<queue>", cxx98},
> /* <set>. */
> - {"set", "<set>"},
> - {"multiset", "<set>"},
> + {"set", "<set>", cxx98},
> + {"multiset", "<set>", cxx98},
> + /* <shared_mutex>. */
> + {"shared_lock", "<shared_mutex>", cxx14},
> + {"shared_mutex", "<shared_mutex>", cxx17},
> + {"shared_timed_mutex", "<shared_mutex>", cxx14},
> /* <sstream>. */
> - {"basic_stringbuf", "<sstream>"},
> - {"basic_istringstream", "<sstream>"},
> - {"basic_ostringstream", "<sstream>"},
> - {"basic_stringstream", "<sstream>"},
> + {"basic_stringbuf", "<sstream>", cxx98},
> + {"basic_istringstream", "<sstream>", cxx98},
> + {"basic_ostringstream", "<sstream>", cxx98},
> + {"basic_stringstream", "<sstream>", cxx98},
> + {"istringstream", "<sstream>", cxx98},
> + {"ostringstream", "<sstream>", cxx98},
> + {"stringstream", "<sstream>", cxx98},
> /* <stack>. */
> - {"stack", "<stack>"},
> - /* <tuple>. */
> - {"make_tuple", "<tuple>"},
> - {"tuple", "<tuple>"},
> + {"stack", "<stack>", cxx98},
> /* <string>. */
> - {"string", "<string>"},
> - {"wstring", "<string>"},
> - {"u16string", "<string>"},
> - {"u32string", "<string>"},
> + {"basic_string", "<string>", cxx98},
> + {"string", "<string>", cxx98},
> + {"wstring", "<string>", cxx98},
> + {"u16string", "<string>", cxx11},
> + {"u32string", "<string>", cxx11},
> + /* <string_view>. */
> + {"string_view", "<string_view>", cxx17},
> + /* <thread>. */
> + {"thread", "<thread>", cxx11},
> + /* <tuple>. */
> + {"make_tuple", "<tuple>", cxx11},
> + {"tuple", "<tuple>", cxx11},
> + {"tuple_element", "<tuple>", cxx11},
> + {"tuple_size", "<tuple>", cxx11},
> /* <unordered_map>. */
> - {"unordered_map", "<unordered_map>"}, // C++11
> - {"unordered_multimap", "<unordered_map>"}, // C++11
> + {"unordered_map", "<unordered_map>", cxx11},
> + {"unordered_multimap", "<unordered_map>", cxx11},
> /* <unordered_set>. */
> - {"unordered_set", "<unordered_set>"}, // C++11
> - {"unordered_multiset", "<unordered_set>"}, // C++11
> + {"unordered_set", "<unordered_set>", cxx11},
> + {"unordered_multiset", "<unordered_set>", cxx11},
> /* <utility>. */
> - {"forward", "<utility>"},
> - {"make_pair", "<utility>"},
> - {"move", "<utility>"},
> - {"pair", "<utility>"},
> + {"declval", "<utility>", cxx11},
> + {"forward", "<utility>", cxx11},
> + {"make_pair", "<utility>", cxx98},
> + {"move", "<utility>", cxx11},
> + {"pair", "<utility>", cxx98},
> + /* <variant>. */
> + {"variant", "<variant>", cxx17},
> + {"visit", "<variant>", cxx17},
> /* <vector>. */
> - {"vector", "<vector>"},
> + {"vector", "<vector>", cxx98},
> };
> const size_t num_hints = sizeof (hints) / sizeof (hints[0]);
> for (size_t i = 0; i < num_hints; i++)
> {
> if (strcmp (name, hints[i].name) == 0)
> - return hints[i].header;
> + return &hints[i];
> }
> return NULL;
> }
>
> +/* Describe DIALECT. */
> +
> +static const char *
> +get_cxx_dialect_name (enum cxx_dialect dialect)
> +{
> + switch (dialect)
> + {
> + default:
> + gcc_unreachable ();
> + case cxx98:
> + return "C++98";
> + case cxx11:
> + return "C++11";
> + case cxx14:
> + return "C++14";
> + case cxx17:
> + return "C++17";
> + case cxx2a:
> + return "C++2a";
> + }
> +}
> +
> /* Suggest pertinent header files for NAME at LOCATION, for common
> names within the "std" namespace.
> Return true iff a suggestion was offered. */
> @@ -5549,16 +5659,26 @@ maybe_suggest_missing_std_header (location_t location, tree name)
> gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
>
> const char *name_str = IDENTIFIER_POINTER (name);
> - const char *header_hint = get_std_name_hint (name_str);
> + const std_name_hint *header_hint = get_std_name_hint (name_str);
> if (!header_hint)
> return false;
>
> gcc_rich_location richloc (location);
> - maybe_add_include_fixit (&richloc, header_hint);
> - inform (&richloc,
> - "%<std::%s%> is defined in header %qs;"
> - " did you forget to %<#include %s%>?",
> - name_str, header_hint, header_hint);
> + if (cxx_dialect >= header_hint->min_dialect)
> + {
> + const char *header = header_hint->header;
> + maybe_add_include_fixit (&richloc, header);
> + inform (&richloc,
> + "%<std::%s%> is defined in header %qs;"
> + " did you forget to %<#include %s%>?",
> + name_str, header, header);
> + }
> + else
> + {
> + inform (&richloc,
> + "%<std::%s%> is only available from %s onwards",
> + name_str, get_cxx_dialect_name (header_hint->min_dialect));
> + }
> return true;
> }
>
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
> index 100bcc0..d9eeb42 100644
> --- a/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
> @@ -60,3 +60,16 @@ void test_move(T&& arg)
> // { dg-message "'#include <utility>'" "" { target *-*-* } .-1 }
> // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
> }
> +
> +void test_array ()
> +{
> + std::array a; // { dg-error ".array. is not a member of .std." }
> + // { dg-message ".std::array. is defined in header .<array>.; did you forget to .#include <array>.?" "" { target *-*-* } .-1 }
> +}
> +
> +void test_tuple ()
> +{
> + std::tuple<int,float> p; // { dg-error ".tuple. is not a member of .std." }
> + // { dg-message ".std::tuple. is defined in header .<tuple>.; did you forget to .#include <tuple>.?" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
> +}
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-8.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
> new file mode 100644
> index 0000000..68b2082
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
> @@ -0,0 +1,44 @@
> +/* Verify that we don't offer #include suggestions for things that
> + aren't yet available due to the C++ dialect in use. */
> +// { dg-do compile { target c++98_only } }
> +
> +#include <memory>
> +
> +template<class T>
> +void test_make_shared ()
> +{
> + std::make_shared<T>(); // { dg-error "'make_shared' is not a member of 'std'" }
> + // { dg-message "'std::make_shared' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
> + // { dg-error "expected primary-expression before '\\)' token" "" { target *-*-* } .-3 }
> +}
> +
> +void test_array ()
> +{
> + std::array a; // { dg-error "'array' is not a member of 'std'" }
> + // { dg-message "'std::array' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
> +}
> +
> +void test_tuple ()
> +{
> + std::tuple<int,float> p; // { dg-error "'tuple' is not a member of 'std'" }
> + // { dg-message "'std::tuple' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before 'int'" "" { target *-*-* } .-2 }
> +}
> +
> +/* Since C++14. */
> +std::shared_timed_mutex m; // { dg-error "'shared_timed_mutex' in namespace 'std' does not name a type" }
> +// { dg-message "'std::shared_timed_mutex' is only available from C\\+\\+14 onwards" "" { target *-*-* } .-1 }
> +
> +/* Since C++17: */
> +std::string_view sv; // { dg-error "'string_view' in namespace 'std' does not name a type" }
> +// { dg-message "'std::string_view' is only available from C\\+\\+17 onwards" "" { target *-*-* } .-1 }
> +
> +/* Verify interaction with "using namespace std;". */
> +using namespace std;
> +void test_via_using_directive ()
> +{
> + shared_ptr<int> p; // { dg-error "'shared_ptr' was not declared in this scope" }
> + // { dg-message "'std::shared_ptr' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before 'int'" "" { target *-*-* } .-2 }
> +}
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include.C b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
> index 5452760..0fcc72b 100644
> --- a/gcc/testsuite/g++.dg/lookup/missing-std-include.C
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
> @@ -13,9 +13,6 @@ void test (void)
> std::cin >> i; // { dg-error ".cin. is not a member of .std." }
> // { dg-message ".std::cin. is defined in header .<iostream>.; did you forget to .#include <iostream>.?" "" { target *-*-* } .-1 }
>
> - std::array a; // { dg-error ".array. is not a member of .std." }
> - // { dg-message ".std::array. is defined in header .<array>.; did you forget to .#include <array>.?" "" { target *-*-* } .-1 }
> -
> std::deque a; // { dg-error ".deque. is not a member of .std." }
> // { dg-message ".std::deque. is defined in header .<deque>.; did you forget to .#include <deque>.?" "" { target *-*-* } .-1 }
>
> @@ -30,8 +27,4 @@ void test (void)
> std::pair<int,float> p; // { dg-error ".pair. is not a member of .std." }
> // { dg-message ".std::pair. is defined in header .<utility>.; did you forget to .#include <utility>.?" "" { target *-*-* } .-1 }
> // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
> -
> - std::tuple<int,float> p; // { dg-error ".tuple. is not a member of .std." }
> - // { dg-message ".std::tuple. is defined in header .<tuple>.; did you forget to .#include <tuple>.?" "" { target *-*-* } .-1 }
> - // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
> }
> --
> 1.8.5.3
>
next prev parent reply other threads:[~2018-03-29 19:27 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-22 23:37 [PATCH 0/4] Improvements to #include suggestions David Malcolm
2018-03-22 23:37 ` [PATCH 1/4] More #include suggestions (PR c++/84269) David Malcolm
2018-03-29 19:25 ` Jason Merrill
2018-03-22 23:37 ` [PATCH 4/4] C++: more std header hints; filter on C++ dialect " David Malcolm
2018-03-29 19:30 ` Jason Merrill [this message]
2018-03-22 23:37 ` [PATCH 3/4] C++: suggest missing headers for implicit use of "std" (PR c++/85021) David Malcolm
2018-03-29 19:27 ` Jason Merrill
2018-04-05 23:33 ` [PATCH v2] " David Malcolm
2018-04-06 15:21 ` Jason Merrill
2018-03-23 0:35 ` [PATCH 2/4] C: Ensure that we print include hints in -Wimplicit-function-declaration David Malcolm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CADzB+2kQ60V4_sAxYAShDkJBYaC-q-f1NA3NuMXmNkfsNXBFEw@mail.gmail.com \
--to=jason@redhat.com \
--cc=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).