c++: Use constexpr to avoid wrong -Wsign-compare (PR90691).
authorJason Merrill <jason@redhat.com>
Mon, 27 Jan 2020 03:58:32 +0000 (22:58 -0500)
committerJason Merrill <jason@redhat.com>
Sat, 8 Feb 2020 16:14:58 +0000 (11:14 -0500)
We would like to do constexpr evaluation to avoid false positives on
warnings, but constexpr evaluation can involve function body copying that
changes DECL_UID, which breaks -fcompare-debug.  So let's remember
that we need to avoid that.

PR c++/90691
* expr.c (fold_for_warn): Call maybe_constant_value.
* constexpr.c (struct constexpr_ctx): Add uid_sensitive field.
(maybe_constant_value): Add uid_sensitive parm.
(get_fundef_copy): Don't copy if it's true.
(cxx_eval_call_expression): Don't instantiate if it's true.
(cxx_eval_outermost_constant_expr): Likewise.

gcc/cp/ChangeLog
gcc/cp/constexpr.c
gcc/cp/cp-tree.h
gcc/cp/expr.c
gcc/testsuite/g++.dg/warn/Walways-true-3.C
gcc/testsuite/g++.dg/warn/Wsign-compare-9.C [new file with mode: 0644]

index 3e5e547fd87aa3fbc825ba6f7ebaaf029de369e6..dff6a170580d30dfb50b2d734e4064c8485bdfa0 100644 (file)
@@ -1,5 +1,13 @@
 2020-02-08  Jason Merrill  <jason@redhat.com>
 
+       PR c++/90691
+       * expr.c (fold_for_warn): Call maybe_constant_value.
+       * constexpr.c (struct constexpr_ctx): Add uid_sensitive bit-field.
+       (maybe_constant_value): Add uid_sensitive parm.
+       (get_fundef_copy): Don't copy if it's true.
+       (cxx_eval_call_expression): Don't instantiate if it's true.
+       (cxx_eval_outermost_constant_expr): Likewise.
+
        PR c++/92852
        * constexpr.c (maybe_constant_value): Don't unshare if the cached
        value is the same as the argument.
index 1b35bc107e95b2bb49bc76f6a58204cbde7eca2f..192ba566427df22dababf95b4616b40b61d701f8 100644 (file)
@@ -1074,13 +1074,18 @@ struct constexpr_ctx {
   /* If inside SWITCH_EXPR.  */
   constexpr_switch_state *css_state;
 
-  /* Whether we should error on a non-constant expression or fail quietly.  */
+  /* Whether we should error on a non-constant expression or fail quietly.
+     This flag needs to be here, but some of the others could move to global
+     if they get larger than a word.  */
   bool quiet;
   /* Whether we are strictly conforming to constant expression rules or
      trying harder to get a constant value.  */
   bool strict;
   /* Whether __builtin_is_constant_evaluated () should be true.  */
   bool manifestly_const_eval;
+  /* Whether we want to avoid doing anything that will cause extra DECL_UID
+     generation.  */
+  bool uid_sensitive;
 };
 
 /* A table of all constexpr calls that have been evaluated by the
@@ -1145,7 +1150,7 @@ static GTY(()) hash_map<tree, tree> *fundef_copies_table;
    is parms, TYPE is result.  */
 
 static tree
-get_fundef_copy (constexpr_fundef *fundef)
+get_fundef_copy (const constexpr_ctx *ctx, constexpr_fundef *fundef)
 {
   tree copy;
   bool existed;
@@ -1162,6 +1167,9 @@ get_fundef_copy (constexpr_fundef *fundef)
     }
   else if (*slot == NULL_TREE)
     {
+      if (ctx->uid_sensitive)
+       return NULL_TREE;
+
       /* We've already used the function itself, so make a copy.  */
       copy = build_tree_list (NULL, NULL);
       tree saved_body = DECL_SAVED_TREE (fundef->decl);
@@ -2232,6 +2240,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 
   /* We can't defer instantiating the function any longer.  */
   if (!DECL_INITIAL (fun)
+      && !ctx->uid_sensitive
       && DECL_TEMPLOID_INSTANTIATION (fun))
     {
       location_t save_loc = input_location;
@@ -2378,13 +2387,12 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
          gcc_assert (at_eof >= 2 && ctx->quiet);
          *non_constant_p = true;
        }
-      else
+      else if (tree copy = get_fundef_copy (ctx, new_call.fundef))
        {
          tree body, parms, res;
          releasing_vec ctors;
 
          /* Reuse or create a new unshared copy of this function's body.  */
-         tree copy = get_fundef_copy (new_call.fundef);
          body = TREE_PURPOSE (copy);
          parms = TREE_VALUE (copy);
          res = TREE_TYPE (copy);
@@ -2522,6 +2530,9 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
                    }
            }
        }
+      else
+       /* Couldn't get a function copy to evaluate.  */
+       *non_constant_p = true;
 
       if (result == error_mark_node)
        *non_constant_p = true;
@@ -6275,7 +6286,8 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
                                  bool strict = true,
                                  bool manifestly_const_eval = false,
                                  bool constexpr_dtor = false,
-                                 tree object = NULL_TREE)
+                                 tree object = NULL_TREE,
+                                 bool uid_sensitive = false)
 {
   auto_timevar time (TV_CONSTEXPR);
 
@@ -6285,7 +6297,8 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
   constexpr_global_ctx global_ctx;
   constexpr_ctx ctx = { &global_ctx, NULL, NULL, NULL, NULL, NULL,
                        allow_non_constant, strict,
-                       manifestly_const_eval || !allow_non_constant };
+                       manifestly_const_eval || !allow_non_constant,
+                       uid_sensitive };
 
   tree type = initialized_type (t);
   tree r = t;
@@ -6375,7 +6388,8 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
   auto_vec<tree, 16> cleanups;
   global_ctx.cleanups = &cleanups;
 
-  instantiate_constexpr_fns (r);
+  if (!uid_sensitive)
+    instantiate_constexpr_fns (r);
   r = cxx_eval_constant_expression (&ctx, r,
                                    false, &non_constant_p, &overflow_p);
 
@@ -6623,12 +6637,14 @@ fold_simple (tree t)
    Otherwise, if T does not have TREE_CONSTANT set, returns T.
    Otherwise, returns a version of T without TREE_CONSTANT.
    MANIFESTLY_CONST_EVAL is true if T is manifestly const-evaluated
-   as per P0595.  */
+   as per P0595.  UID_SENSITIVE is true if we can't do anything that
+   would affect DECL_UID ordering.  */
 
 static GTY((deletable)) hash_map<tree, tree> *cv_cache;
 
 tree
-maybe_constant_value (tree t, tree decl, bool manifestly_const_eval)
+maybe_constant_value (tree t, tree decl, bool manifestly_const_eval,
+                     bool uid_sensitive)
 {
   tree r;
 
@@ -6646,7 +6662,8 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval)
     return t;
 
   if (manifestly_const_eval)
-    return cxx_eval_outermost_constant_expr (t, true, true, true, false, decl);
+    return cxx_eval_outermost_constant_expr (t, true, true, true, false,
+                                            decl, uid_sensitive);
 
   if (cv_cache == NULL)
     cv_cache = hash_map<tree, tree>::create_ggc (101);
@@ -6658,7 +6675,10 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval)
          r = unshare_expr_without_location (r);
          protected_set_expr_location (r, EXPR_LOCATION (t));
        }
-      return r;
+      if (r != t || TREE_CONSTANT (t) || !manifestly_const_eval)
+       return r;
+      /* If we cached this as non-constant and we need a constant value, try
+        again; we might have failed before due to UID_SENSITIVE.  */
     }
 
   r = cxx_eval_outermost_constant_expr (t, true, true, false, false, decl);
index c46d1e92b3b0fbcc4f3cbf1d6e97657f6644c96e..037d3b6453825508f226cb57aff6ddc00e173874 100644 (file)
@@ -7930,7 +7930,7 @@ extern bool require_potential_rvalue_constant_expression (tree);
 extern tree cxx_constant_value                 (tree, tree = NULL_TREE);
 extern void cxx_constant_dtor                  (tree, tree);
 extern tree cxx_constant_init                  (tree, tree = NULL_TREE);
-extern tree maybe_constant_value               (tree, tree = NULL_TREE, bool = false);
+extern tree maybe_constant_value               (tree, tree = NULL_TREE, bool = false, bool = false);
 extern tree maybe_constant_init                        (tree, tree = NULL_TREE, bool = false);
 extern tree fold_non_dependent_expr            (tree,
                                                 tsubst_flags_t = tf_warning_or_error,
index cfc1c7b64cac0ac8e7cec4f8328f3c7a571873be..04e4418c671bfa389cafdce9e596c5d333f6afaf 100644 (file)
@@ -397,6 +397,8 @@ fold_for_warn (tree x)
       else
        return f;
     }
+  else if (cxx_dialect >= cxx11)
+    x = maybe_constant_value (x, NULL_TREE, false, true);
 
   return c_fully_fold (x, /*for_init*/false, /*maybe_constp*/NULL);
 }
index 0291328d415eccbaace32265daf31fde57bb404c..dd02843857b824d645bf07723a41674ff111dc1d 100644 (file)
@@ -17,7 +17,7 @@ bar (int &a)
   if (&b) // { dg-warning "7:the compiler can assume that the address of" }
     foo ();
 
-  if (!&c) // { dg-warning "8:the compiler can assume that the address of" }
+  if (!&c) // { dg-warning "8:the address of" }
     foo ();
 
   if (!&(int &)(int &)a) // { dg-warning "8:the compiler can assume that the address of" }
@@ -29,7 +29,7 @@ bar (int &a)
   if (&b != 0) // { dg-warning "10:the compiler can assume that the address of" }
     foo ();
 
-  if (0 == &(int &)(int &)c) // { dg-warning "9:the compiler can assume that the address of" }
+  if (0 == &(int &)(int &)c) // { dg-warning "9:the address of" }
     foo ();
 
   if (&a != (int *)0) // { dg-warning "10:the compiler can assume that the address of" }
diff --git a/gcc/testsuite/g++.dg/warn/Wsign-compare-9.C b/gcc/testsuite/g++.dg/warn/Wsign-compare-9.C
new file mode 100644 (file)
index 0000000..bf3514c
--- /dev/null
@@ -0,0 +1,22 @@
+// PR c++/90691
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -Wsign-compare }
+
+struct S {
+  int a;
+
+  constexpr S();
+  explicit constexpr S(int a_) : a(a_) {}
+};
+
+constexpr S b = S(12);
+
+template <const S& e>
+bool c(unsigned int d) {
+  return d >= e.a;
+}
+
+bool test(unsigned int d);
+bool test(unsigned int d) {
+    return c<b>(d);
+}