[gcc r12-3231] c++: Add warning about missing 'requires'

[gcc r12-3231] c++: Add warning about missing 'requires'

[Jason Merrill]

https://gcc.gnu.org/g:e18e56c76be35e6a799e07a01c24e0fff3eb1978

commit r12-3231-ge18e56c76be35e6a799e07a01c24e0fff3eb1978
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Aug 27 17:28:28 2021 -0400

    c++: Add warning about missing 'requires'
    
    I noticed that concepts-lambda14.C had two useless requires-expressions:
    
      static_assert(requires { C<T>; });
    
    always succeeds, because C<T> is always a valid expression for any type,
    regardless of whether C is satisfied for a particular type.  Presumably the
    user means
    
      static_assert(requires { requires C<T>; });
    
    to make the C<T> a nested-requirement.  Of course,
    
      static_assert(C<T>);
    
    is much simpler and means the same thing; this is more relevant in the
    middle of a longer requires-expression, such as the bug this warning found
    in cmcstl2:
    
      template<class I>
      META_CONCEPT input_iterator =
              input_or_output_iterator<I> &&
              readable<I> &&
              requires(I& i, const I& ci) {
                      typename iterator_category_t<I>;
                      derived_from<iterator_category_t<I>, input_iterator_tag>;
                      i++;
              };
    
    where 'requires' is missing before 'derived_from'.
    
    gcc/ChangeLog:
    
            * doc/invoke.texi: Document -Wmissing-requires.
    
    gcc/c-family/ChangeLog:
    
            * c.opt: Add -Wmissing-requires.
    
    gcc/cp/ChangeLog:
    
            * parser.c (cp_parser_simple_requirement): Warn about missing
            requires.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp2a/concepts-lambda14.C: Add expected warnings.

Diff:
---
 gcc/doc/invoke.texi                            | 22 ++++++++++++++++++++++
 gcc/c-family/c.opt                             |  4 ++++
 gcc/cp/parser.c                                | 19 +++++++++++++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C |  4 ++--
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index f7bb193b51d..8969bac664d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8752,6 +8752,28 @@ s x = @{ @};
 This warning is included in @option{-Wextra}.  To get other @option{-Wextra}
 warnings without this one, use @option{-Wextra -Wno-missing-field-initializers}.
 
+@item -Wno-missing-requires
+@opindex Wmissing-requires
+@opindex Wno-missing-requires
+
+By default, the compiler warns about a concept-id appearing as a C++20 simple-requirement:
+
+@smallexample
+bool satisfied = requires @{ C<T> @};
+@end smallexample
+
+Here @samp{satisfied} will be true if @samp{C<T>} is a valid
+expression, which it is for all T.  Presumably the user meant to write
+
+@smallexample
+bool satisfied = requires @{ requires C<T> @};
+@end smallexample
+
+so @samp{satisfied} is only true if concept @samp{C} is satisfied for
+type @samp{T}.
+
+This warning can be disabled with @option{-Wno-missing-requires}.
+
 @item -Wno-multichar
 @opindex Wno-multichar
 @opindex Wmultichar
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 91929706aff..c5fe90003f2 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -839,6 +839,10 @@ Wmissing-field-initializers
 C ObjC C++ ObjC++ Var(warn_missing_field_initializers) Warning EnabledBy(Wextra)
 Warn about missing fields in struct initializers.
 
+Wmissing-requires
+C++ ObjC++ Var(warn_missing_requires) Init(1) Warning
+Warn about likely missing requires keyword.
+
 Wmultistatement-macros
 C ObjC C++ ObjC++ Var(warn_multistatement_macros) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about unsafe macros expanding to multiple statements used as a body of a clause such as if, else, while, switch, or for.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index a959c71dfa3..797e70ba5bb 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -29911,6 +29911,25 @@ cp_parser_simple_requirement (cp_parser *parser)
   if (expr.get_location() == UNKNOWN_LOCATION)
     expr.set_location (start);
 
+  for (tree t = expr; ; )
+    {
+      if (TREE_CODE (t) == TRUTH_ANDIF_EXPR
+	  || TREE_CODE (t) == TRUTH_ORIF_EXPR)
+	{
+	  t = TREE_OPERAND (t, 0);
+	  continue;
+	}
+      if (concept_check_p (t))
+	{
+	  gcc_rich_location richloc (get_start (start));
+	  richloc.add_fixit_insert_before (start, "requires ");
+	  warning_at (&richloc, OPT_Wmissing_requires, "testing "
+		      "if a concept-id is a valid expression; add "
+		      "%<requires%> to check satisfaction");
+	}
+      break;
+    }
+
   return finish_simple_requirement (expr.get_location (), expr);
 }
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C
index bdc893da857..02b6b6a8438 100644
--- a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C
@@ -11,9 +11,9 @@ void foo() noexcept(!__is_same(T, void)) { }
 
 template<typename U>
 auto f() {
-  return []<typename T>(T, bool a = requires { C<T>; }){
+  return []<typename T>(T, bool a = requires { C<T>; }){ // { dg-warning Wmissing-requires }
     static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { dg-error "assert" }
-    static_assert(requires { C<T>; });
+    static_assert(requires { C<T>; }); // { dg-warning Wmissing-requires }
     static_assert(requires { { foo<T>() } noexcept -> C; });
     static_assert(!requires { typename T::blah; }); // { dg-error "assert" }
     return 0;