Check NoDiscard together with Deprecated

TimWolla · TimWolla · commit 07c0b6526a9f · 2025-01-27T17:06:50.000+01:00
diff --git a/Zend/tests/attributes/nodiscard/001.phpt b/Zend/tests/attributes/nodiscard/001.phpt
@@ -63,24 +63,24 @@ $closure2();
 
 ?>
 --EXPECTF--
-Warning: (F)The return value of function test() is expected to be consumed in %s on line %d
+Warning: (B)The return value of function test() is expected to be consumed in %s on line %d
 
-Warning: (F)The return value of function test2() is expected to be consumed, this is important in %s on line %d
+Warning: (B)The return value of function test2() is expected to be consumed, this is important in %s on line %d
 
-Warning: (E)The return value of function test3() is expected to be consumed in %s on line %d
+Warning: (B)The return value of function test3() is expected to be consumed in %s on line %d
 
-Warning: (F)The return value of function test() is expected to be consumed in %s on line %d
+Warning: (A)The return value of function test() is expected to be consumed in %s on line %d
 
-Warning: (F)The return value of function test() is expected to be consumed in %s on line %d
+Warning: (A)The return value of function test() is expected to be consumed in %s on line %d
 
-Warning: (F)The return value of method Clazz::test() is expected to be consumed in %s on line %d
+Warning: (A)The return value of method Clazz::test() is expected to be consumed in %s on line %d
 
-Warning: (F)The return value of method Clazz::test2() is expected to be consumed, this is important in %s on line %d
+Warning: (A)The return value of method Clazz::test2() is expected to be consumed, this is important in %s on line %d
 
-Warning: (F)The return value of method Clazz::test3() is expected to be consumed in %s on line %d
+Warning: (B)The return value of method Clazz::test3() is expected to be consumed in %s on line %d
 
-Warning: (F)The return value of method Clazz::test() is expected to be consumed in %s on line %d
+Warning: (A)The return value of method Clazz::test() is expected to be consumed in %s on line %d
 
-Warning: (F)The return value of function {closure:%s:%d}() is expected to be consumed in %s on line %d
+Warning: (A)The return value of function {closure:%s:%d}() is expected to be consumed in %s on line %d
 
-Warning: (F)The return value of function {closure:%s:%d}() is expected to be consumed in %s on line %d
+Warning: (A)The return value of function {closure:%s:%d}() is expected to be consumed in %s on line %d
diff --git a/Zend/tests/attributes/nodiscard/002.phpt b/Zend/tests/attributes/nodiscard/002.phpt
@@ -33,12 +33,12 @@ $cls('foo');
 
 ?>
 --EXPECTF--
+Warning: (A)The return value of method Clazz::test() is expected to be consumed in %s on line %d
 __call(test)
 
-Warning: (F)The return value of method Clazz::__call() is expected to be consumed in %s on line %d
+Warning: (A)The return value of method Clazz::test() is expected to be consumed in %s on line %d
 __callStatic(test)
 
-Warning: (F)The return value of method Clazz::__callStatic() is expected to be consumed in %s on line %d
+Warning: (A)The return value of method Clazz::__invoke() is expected to be consumed in %s on line %d
 __invoke(foo)
 
-Warning: (F)The return value of method Clazz::__invoke() is expected to be consumed in %s on line %d
diff --git a/Zend/tests/attributes/nodiscard/003.phpt b/Zend/tests/attributes/nodiscard/003.phpt
@@ -19,4 +19,4 @@ $cls->test();
 
 ?>
 --EXPECTF--
-Warning: (F)The return value of method Clazz::test() is expected to be consumed in %s on line %d
+Warning: (A)The return value of method Clazz::test() is expected to be consumed in %s on line %d
diff --git a/Zend/tests/attributes/nodiscard/005.phpt b/Zend/tests/attributes/nodiscard/005.phpt
@@ -12,6 +12,6 @@ $date->setTimestamp(0);
 
 ?>
 --EXPECTF--
-Warning: (C)The return value of function flock() is expected to be consumed, as locking the stream might have failed in %s on line %d
+Warning: (B)The return value of function flock() is expected to be consumed, as locking the stream might have failed in %s on line %d
 
 Warning: (A)The return value of method DateTimeImmutable::setTimestamp() is expected to be consumed, as DateTimeImmutable::setTimestamp() does not modify the object itself in %s on line %d
diff --git a/Zend/tests/attributes/nodiscard/006.phpt b/Zend/tests/attributes/nodiscard/006.phpt
@@ -17,4 +17,4 @@ test();
 
 ?>
 --EXPECTF--
-Warning: (G)The return value of function test() is expected to be consumed in %s on line %d
+Warning: (A)The return value of function test() is expected to be consumed in %s on line %d
diff --git a/Zend/tests/attributes/nodiscard/007.phpt b/Zend/tests/attributes/nodiscard/007.phpt
@@ -14,8 +14,8 @@ fclose($f);
 ?>
 --EXPECTF--
 <!-- internal enter tmpfile() -->
-<!-- internal enter flock() -->
 <!-- internal enter NoDiscard::__construct() -->
 
 Warning: (A)The return value of function flock() is expected to be consumed, as locking the stream might have failed in %s on line %d
+<!-- internal enter flock() -->
 <!-- internal enter fclose() -->
diff --git a/Zend/tests/attributes/nodiscard/008.phpt b/Zend/tests/attributes/nodiscard/008.phpt
@@ -15,4 +15,4 @@ test();
 --EXPECTF--
 Deprecated: Function test() is deprecated in %s on line %d
 
-Warning: (F)The return value of function test() is expected to be consumed in %s on line %d
+Warning: (B)The return value of function test() is expected to be consumed in %s on line %d
diff --git a/Zend/tests/attributes/nodiscard/throwing_error_handler_001.phpt b/Zend/tests/attributes/nodiscard/throwing_error_handler_001.phpt
@@ -31,5 +31,5 @@ try {
 
 ?>
 --EXPECT--
-Caught: (F)The return value of function test() is expected to be consumed
-Caught: (F)The return value of function test2() is expected to be consumed
+Caught: (B)The return value of function test() is expected to be consumed
+Caught: (B)The return value of function test2() is expected to be consumed
diff --git a/Zend/tests/attributes/nodiscard/type_validation_001.phpt b/Zend/tests/attributes/nodiscard/type_validation_001.phpt
@@ -12,4 +12,4 @@ test();
 
 ?>
 --EXPECTF--
-Warning: (F)The return value of function test() is expected to be consumed, 1234 in %s on line %d
+Warning: (B)The return value of function test() is expected to be consumed, 1234 in %s on line %d
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -3927,15 +3927,15 @@ ZEND_API uint8_t zend_get_call_op(const zend_op *init_op, zend_function *fbc) /*
 		ZEND_ASSERT(!(fbc->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE));
 		if (fbc->type == ZEND_INTERNAL_FUNCTION && !(CG(compiler_options) & ZEND_COMPILE_IGNORE_INTERNAL_FUNCTIONS)) {
 			if (init_op->opcode == ZEND_INIT_FCALL && !zend_execute_internal) {
-				if (!(fbc->common.fn_flags & ZEND_ACC_DEPRECATED)) {
+				if (!(fbc->common.fn_flags & (ZEND_ACC_DEPRECATED|ZEND_ACC_NODISCARD))) {
 					return ZEND_DO_ICALL;
 				} else {
 					return ZEND_DO_FCALL_BY_NAME;
 				}
 			}
 		} else if (!(CG(compiler_options) & ZEND_COMPILE_IGNORE_USER_FUNCTIONS)){
 			if (zend_execute_ex == execute_ex) {
-				if (!(fbc->common.fn_flags & ZEND_ACC_DEPRECATED)) {
+				if (!(fbc->common.fn_flags & (ZEND_ACC_DEPRECATED|ZEND_ACC_NODISCARD))) {
 					return ZEND_DO_UCALL;
 				} else {
 					return ZEND_DO_FCALL_BY_NAME;
diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
@@ -1617,7 +1617,7 @@ ZEND_API zend_function *zend_get_call_trampoline_func(const zend_class_entry *ce
 	func->fn_flags = ZEND_ACC_CALL_VIA_TRAMPOLINE
 		| ZEND_ACC_PUBLIC
 		| ZEND_ACC_VARIADIC
-		| (fbc->common.fn_flags & (ZEND_ACC_RETURN_REFERENCE|ZEND_ACC_DEPRECATED));
+		| (fbc->common.fn_flags & (ZEND_ACC_RETURN_REFERENCE|ZEND_ACC_DEPRECATED|ZEND_ACC_NODISCARD));
 	if (fbc->common.attributes) {
 		func->attributes = fbc->common.attributes;
 		GC_TRY_ADDREF(func->attributes);
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
@@ -2936,12 +2936,6 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
 			OBJ_RELEASE(ZEND_CLOSURE_OBJECT(EX(func)));
 		}
 		EG(vm_stack_top) = (zval*)execute_data;
-
-		if (UNEXPECTED((EX(func)->common.fn_flags & ZEND_ACC_NODISCARD) != 0) && !EX(return_value)) {
-			zend_function *fbc = EX(func);
-			zend_nodiscard_function("(F)", fbc);
-		}
-
 		execute_data = EX(prev_execute_data);
 
 		if (UNEXPECTED(EG(exception) != NULL)) {
@@ -2977,11 +2971,6 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
 		}
 
 		old_execute_data = execute_data;
-		if (UNEXPECTED((EX(func)->common.fn_flags & ZEND_ACC_NODISCARD) != 0) && !EX(return_value)) {
-			zend_function *fbc = EX(func);
-			zend_nodiscard_function("(E)", fbc);
-		}
-
 		execute_data = EX(prev_execute_data);
 		zend_vm_stack_free_call_frame_ex(call_info, old_execute_data);
 
@@ -3001,8 +2990,6 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
 		destroy_op_array(&EX(func)->op_array);
 		efree_size(EX(func), sizeof(zend_op_array));
 		old_execute_data = execute_data;
-		ZEND_ASSERT(EX(return_value));
-
 		execute_data = EG(current_execute_data) = EX(prev_execute_data);
 		zend_vm_stack_free_call_frame_ex(call_info, old_execute_data);
 
@@ -3023,12 +3010,6 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
 	} else {
 		if (EXPECTED((call_info & ZEND_CALL_CODE) == 0)) {
 			EG(current_execute_data) = EX(prev_execute_data);
-
-			if (UNEXPECTED((EX(func)->common.fn_flags & ZEND_ACC_NODISCARD) != 0) && !EX(return_value)) {
-				zend_function *fbc = EX(func);
-				zend_nodiscard_function("(G)", fbc);
-			}
-
 			i_free_compiled_variables(execute_data);
 #ifdef ZEND_PREFER_RELOAD
 			call_info = EX_CALL_INFO();
@@ -4127,9 +4108,6 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
 	}
 
 	if (!RETURN_VALUE_USED(opline)) {
-		if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_NODISCARD) != 0)) {
-			zend_nodiscard_function("(C)", fbc);
-		}
 		i_zval_ptr_dtor(ret);
 	}
 
@@ -4178,8 +4156,13 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL,OBSERVER))
 	SAVE_OPLINE();
 	EX(call) = call->prev_execute_data;
 
-	if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_DEPRECATED) != 0)) {
-		zend_deprecated_function(fbc);
+	if (UNEXPECTED((fbc->common.fn_flags & (ZEND_ACC_DEPRECATED|ZEND_ACC_NODISCARD)) != 0)) {
+		if ((fbc->common.fn_flags & ZEND_ACC_DEPRECATED) != 0) {
+			zend_deprecated_function(fbc);
+		}
+		if (EG(exception) == NULL && (fbc->common.fn_flags & ZEND_ACC_NODISCARD) != 0 && !RETURN_VALUE_USED(opline)) {
+			zend_nodiscard_function("(B)", fbc);
+		}
 		if (UNEXPECTED(EG(exception) != NULL)) {
 			UNDEF_RESULT();
 			if (!RETURN_VALUE_USED(opline)) {
@@ -4259,9 +4242,6 @@ ZEND_VM_C_LABEL(fcall_by_name_end):
 		}
 
 		if (!RETURN_VALUE_USED(opline)) {
-			if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_NODISCARD) != 0)) {
-				zend_nodiscard_function("(B)", fbc);
-			}
 			i_zval_ptr_dtor(ret);
 		}
 	}
@@ -4285,8 +4265,13 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
 	SAVE_OPLINE();
 	EX(call) = call->prev_execute_data;
 
-	if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_DEPRECATED) != 0)) {
-		zend_deprecated_function(fbc);
+	if (UNEXPECTED((fbc->common.fn_flags & (ZEND_ACC_DEPRECATED|ZEND_ACC_NODISCARD)) != 0)) {
+		if ((fbc->common.fn_flags & ZEND_ACC_DEPRECATED) != 0) {
+			zend_deprecated_function(fbc);
+		}
+		if (EG(exception) == NULL && (fbc->common.fn_flags & ZEND_ACC_NODISCARD) != 0 && !RETURN_VALUE_USED(opline)) {
+			zend_nodiscard_function("(A)", fbc);
+		}
 		if (UNEXPECTED(EG(exception) != NULL)) {
 			if (UNEXPECTED(ZEND_CALL_INFO(call) & ZEND_CALL_CLOSURE)) {
 				OBJ_RELEASE(ZEND_CLOSURE_OBJECT(call->func));
@@ -4376,9 +4361,6 @@ ZEND_VM_C_LABEL(fcall_end):
 		}
 
 		if (!RETURN_VALUE_USED(opline)) {
-			if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_NODISCARD) != 0)) {
-				zend_nodiscard_function("(A)", fbc);
-			}
 			i_zval_ptr_dtor(ret);
 		}
 	}
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
