This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/4] C++: more std header hints; filter on C++ dialect (PR c++/84269)


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
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]