diff mbox series

[committed] d: Add warning for call expression without side effects

Message ID 20231028081121.145230-1-ibuclaw@gdcproject.org
State Accepted
Headers
Series [committed] d: Add warning for call expression without side effects |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Iain Buclaw Oct. 28, 2023, 8:11 a.m. UTC 
  Hi,

In the last merge of the dmd front-end with upstream (r14-4830), this
warning got removed from the semantic passes.  Reimplement the warning
for the code generation pass instead, where it cannot have an effect on
conditional compilation.

Bootstrapped and regression tested on x86_64-linux-gnu/-m32, committed
to mainline.

Regards,
Iain.

---
gcc/d/ChangeLog:

	* d-codegen.cc (call_side_effect_free_p): New function.
	* d-tree.h (CALL_EXPR_WARN_IF_UNUSED): New macro.
	(call_side_effect_free_p): New prototype.
	* expr.cc (ExprVisitor::visit (CallExp *)): Set
	CALL_EXPR_WARN_IF_UNUSED on matched call expressions.
	(ExprVisitor::visit (NewExp *)): Don't dereference the result of an
	allocation call here.
	* toir.cc (add_stmt): Emit warning when call expression added to
	statement list without being used.

gcc/testsuite/ChangeLog:

	* gdc.dg/Wunused_value.d: New test.
---
 gcc/d/d-codegen.cc                   | 54 ++++++++++++++++++++++++++++
 gcc/d/d-tree.h                       |  7 ++++
 gcc/d/expr.cc                        | 13 ++++++-
 gcc/d/toir.cc                        | 32 +++++++++++++++++
 gcc/testsuite/gdc.dg/Wunused_value.d | 29 +++++++++++++++
 5 files changed, 134 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gdc.dg/Wunused_value.d
diff mbox series

Patch

diff --git a/gcc/d/d-codegen.cc b/gcc/d/d-codegen.cc
index 155f5d0d618..8c2e6c70ed4 100644
--- a/gcc/d/d-codegen.cc
+++ b/gcc/d/d-codegen.cc
@@ -2102,6 +2102,60 @@  get_function_type (Type *t)
   return tf;
 }
 
+/* Returns TRUE if calling the function FUNC, or the function or delegate type
+   TYPE is free of side effects.  */
+
+bool
+call_side_effect_free_p (FuncDeclaration *func, Type *type)
+{
+  gcc_assert (func != NULL || type != NULL);
+
+  if (func != NULL)
+    {
+      /* Constructor and invariant calls can't be `pure'.  */
+      if (func->isCtorDeclaration () || func->isInvariantDeclaration ())
+	return false;
+
+      /* Must be a `nothrow' function.  */
+      TypeFunction *tf = func->type->toTypeFunction ();
+      if (!tf->isnothrow ())
+	return false;
+
+      /* Return type can't be `void' or `noreturn', as that implies all work is
+	 done via side effects.  */
+      if (tf->next->ty == TY::Tvoid || tf->next->ty == TY::Tnoreturn)
+	return false;
+
+      /* Only consider it as `pure' if it can't modify its arguments.  */
+      if (func->isPure () == PURE::const_)
+	return true;
+    }
+
+  if (type != NULL)
+    {
+      TypeFunction *tf = get_function_type (type);
+
+      /* Must be a `nothrow` function type.  */
+      if (tf == NULL || !tf->isnothrow ())
+	return false;
+
+      /* Return type can't be `void' or `noreturn', as that implies all work is
+	 done via side effects.  */
+      if (tf->next->ty == TY::Tvoid || tf->next->ty == TY::Tnoreturn)
+	return false;
+
+      /* Delegates that can modify its context can't be `pure'.  */
+      if (type->isTypeDelegate () && tf->isMutable ())
+	return false;
+
+      /* Only consider it as `pure' if it can't modify its arguments.  */
+      if (tf->purity == PURE::const_)
+	return true;
+    }
+
+  return false;
+}
+
 /* Returns TRUE if CALLEE is a plain nested function outside the scope of
    CALLER.  In which case, CALLEE is being called through an alias that was
    passed to CALLER.  */
diff --git a/gcc/d/d-tree.h b/gcc/d/d-tree.h
index 66c2f2465c8..ed26533feb4 100644
--- a/gcc/d/d-tree.h
+++ b/gcc/d/d-tree.h
@@ -47,6 +47,7 @@  typedef Array <Expression *> Expressions;
 
 /* Usage of TREE_LANG_FLAG_?:
    0: METHOD_CALL_EXPR
+   1: CALL_EXPR_WARN_IF_UNUSED (in CALL_EXPR).
 
    Usage of TYPE_LANG_FLAG_?:
    0: TYPE_SHARED
@@ -357,6 +358,11 @@  lang_tree_node
 #define METHOD_CALL_EXPR(NODE) \
   (TREE_LANG_FLAG_0 (NODE))
 
+/* True if the CALL_EXPR is free of side effects, and its return value
+   should not be discarded.  */
+#define CALL_EXPR_WARN_IF_UNUSED(NODE) \
+  (TREE_LANG_FLAG_1 (CALL_EXPR_CHECK (NODE)))
+
 /* True if the type was declared 'shared'.  */
 #define TYPE_SHARED(NODE) \
   (TYPE_LANG_FLAG_0 (NODE))
@@ -594,6 +600,7 @@  extern tree build_bounds_slice_condition (SliceExp *, tree, tree, tree);
 extern bool array_bounds_check (void);
 extern bool checkaction_trap_p (void);
 extern TypeFunction *get_function_type (Type *);
+extern bool call_side_effect_free_p (FuncDeclaration *, Type *);
 extern bool call_by_alias_p (FuncDeclaration *, FuncDeclaration *);
 extern tree d_build_call_expr (FuncDeclaration *, tree, Expressions *);
 extern tree d_build_call (TypeFunction *, tree, tree, Expressions *);
diff --git a/gcc/d/expr.cc b/gcc/d/expr.cc
index 52243e61899..72180b100af 100644
--- a/gcc/d/expr.cc
+++ b/gcc/d/expr.cc
@@ -1714,6 +1714,12 @@  public:
        build the call expression.  */
     tree exp = d_build_call (tf, callee, object, e->arguments);
 
+    /* Record whether the call expression has no side effects, so we can check
+       for an unused return value later.  */
+    if (TREE_CODE (exp) == CALL_EXPR && CALL_EXPR_FN (exp) != NULL_TREE
+	&& call_side_effect_free_p (e->f, e->e1->type))
+      CALL_EXPR_WARN_IF_UNUSED (exp) = 1;
+
     if (returnvalue != NULL_TREE)
       exp = compound_expr (exp, returnvalue);
 
@@ -2338,7 +2344,12 @@  public:
 		new_call = d_save_expr (new_call);
 		se->type = sd->type;
 		se->sym = new_call;
-		result = compound_expr (build_expr (se), new_call);
+
+		/* Setting `se->sym' would mean that the result of the
+		   constructed struct literal expression is `*(new_call)'.
+		   Strip off the indirect reference, as we don't mean to
+		   compute the value yet.  */
+		result = build_address (build_expr (se));
 	      }
 	    else
 	      result = new_call;
diff --git a/gcc/d/toir.cc b/gcc/d/toir.cc
index db6f71babda..f87e094afca 100644
--- a/gcc/d/toir.cc
+++ b/gcc/d/toir.cc
@@ -215,6 +215,38 @@  add_stmt (tree t)
   if (!TREE_SIDE_EFFECTS (t))
     return;
 
+  /* If a call expression has no side effects, and there's no explicit
+     `cast(void)', then issue a warning about the unused return value.  */
+  if (TREE_CODE (t) == INDIRECT_REF)
+    {
+      t = TREE_OPERAND (t, 0);
+
+      if (TREE_CODE (TREE_TYPE (t)) != REFERENCE_TYPE)
+	warning (OPT_Wunused_value, "value computed is not used");
+    }
+
+  if (TREE_CODE (t) == CALL_EXPR && CALL_EXPR_FN (t) != NULL_TREE
+      && CALL_EXPR_WARN_IF_UNUSED (t))
+    {
+      tree callee =  CALL_EXPR_FN (t);
+
+      /* It's a call to a function or function pointer.  */
+      if (TREE_CODE (callee) == ADDR_EXPR
+	  && VAR_OR_FUNCTION_DECL_P (TREE_OPERAND (callee, 0)))
+	callee = TREE_OPERAND (callee, 0);
+
+      /* It's a call to a delegate object.  */
+      if (TREE_CODE (callee) == COMPONENT_REF
+	  && TREE_CODE (TREE_TYPE (TREE_OPERAND (callee, 0))) == RECORD_TYPE
+	  && TYPE_DELEGATE (TREE_TYPE (TREE_OPERAND (callee, 0))))
+	callee = TREE_OPERAND (callee, 0);
+
+      warning (OPT_Wunused_value,
+	       "calling %qE without side effects discards return value "
+	       "of type %qT; prepend a %<cast(void)%> if intentional",
+	       callee, TREE_TYPE (t));
+    }
+
   if (TREE_CODE (t) == COMPOUND_EXPR)
     {
       /* Push out each comma expressions as separate statements.  */
diff --git a/gcc/testsuite/gdc.dg/Wunused_value.d b/gcc/testsuite/gdc.dg/Wunused_value.d
new file mode 100644
index 00000000000..0afc881a7bb
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/Wunused_value.d
@@ -0,0 +1,29 @@ 
+// { dg-do compile }
+// { dg-options "-Wunused-value" }
+
+@safe pure nothrow T t1(T)(T x)
+{
+    return x * x;
+}
+
+nothrow pure int f1(immutable(int)[] a)
+{
+    return 0;
+}
+
+nothrow pure int f2(immutable(int)*  p)
+{
+    return 0;
+}
+
+void test()
+{
+    int x = 3;
+    t1(x); // { dg-warning "without side effects discards return value" }
+
+    auto fp = &t1!int;
+    fp(x); // { dg-warning "without side effects discards return value" }
+
+    f1([]); // { dg-warning "without side effects discards return value" }
+    f2(null); // { dg-warning "without side effects discards return value" }
+}