From patchwork Thu Aug 29 18:18:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13783542 Received: from pb-smtp1.pobox.com (pb-smtp1.pobox.com [64.147.108.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 11B4314B086 for ; Thu, 29 Aug 2024 18:18:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.70 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724955492; cv=none; b=TuuC1DWxPrHRnw2nOj2/EyyOfaH1O/swPalsaP3mE25ZueKIgw9YGWS1sfpb8vZmbQane9RB4/z3R/kUOdP92LW6NRFD8NrX8v6MaE5X999MkR32ce9eB3WTWmd18YvbDzIepEGDHLbW0F1DWrbJyo+DKCz0r4DSRhrspdJRQDY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724955492; c=relaxed/simple; bh=bf+nf+obNI2xRq0a+U4YwRES0cVoSpJbnXhuVPHIHnE=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=f81j35w4Pj5Es7cXJLervc/0sVzEqp2TMpCQJqXgX3T/CbK34Y6/iNseQu1OosyxVTaq+4dhl269lETEj0v1u61YXV0e6smEfiVfznRVkMewNxM0kj2Qv6H2UKilLG514YJAUQ9q7sEK+mGDkcDLtySgWlMKl5SHhq7HW3UbWTo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=VryQf6jK; arc=none smtp.client-ip=64.147.108.70 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="VryQf6jK" Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 065542425B; Thu, 29 Aug 2024 14:18:10 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:in-reply-to:references:date:message-id:mime-version :content-type; s=sasl; bh=bf+nf+obNI2xRq0a+U4YwRES0cVoSpJbnXhuVP HIHnE=; b=VryQf6jKfLtsW/U66+bMl7Oidfn2+QXTFqZBu3OBnuBcfMqt6N3Q2L rainXvPiTaHeYLbMcOWgskVtbBBSy78wo0u2JG+2YWcOrnSXCK4SEQqtx36SGvJG yoZ5qR54oT7eHikCVtgy4BbwmG7ESgnlRzZjrEXvTKtvhASXqRPQE= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id D078A2425A; Thu, 29 Aug 2024 14:18:09 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.94.240]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id 40AEF24259; Thu, 29 Aug 2024 14:18:08 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: Jeff King Cc: git@vger.kernel.org Subject: [PATCH v2] CodingGuidelines: also mention MAYBE_UNUSED In-Reply-To: (Junio C. Hamano's message of "Thu, 29 Aug 2024 11:06:22 -0700") References: <20240829040215.GA4054823@coredump.intra.peff.net> <20240829175215.GA415423@coredump.intra.peff.net> Date: Thu, 29 Aug 2024 11:18:06 -0700 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: 0AD4C4FE-6633-11EF-B96C-2BAEEB2EC81B-77302942!pb-smtp1.pobox.com A function that uses a parameter in one build may lose all uses of the parameter in another build, depending on the configuration. A workaround for such a case, MAYBE_UNUSED, should also be mentioned when we recommend the use of UNUSED to our developers. Keep the addition to the guideline short and document the criteria to choose between UNUSED and MAYBE_UNUSED near their definition. Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 5 +++-- git-compat-util.h | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) Interdiff against v1: diff --git a/git-compat-util.h b/git-compat-util.h index 23307ce780..e4a306dd56 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -196,7 +196,9 @@ struct strbuf; #define _SGI_SOURCE 1 /* - * UNUSED marks a function parameter that is always unused. + * UNUSED marks a function parameter that is always unused. It also + * can be used to annotate a function, a variable, or a type that is + * always unused. * * A callback interface may dictate that a function accepts a * parameter at that position, but the implementation of the function @@ -662,13 +664,14 @@ static inline int git_has_dir_sep(const char *path) /* * MAYBE_UNUSED marks a function parameter that may be unused, but - * whose use is not an error. + * whose use is not an error. It also can be used to annotate a + * function, a variable, or a type that may be unused. * - * Depending on a configuration, all uses of a function parameter may - * become #ifdef'ed away. Marking such a parameter with UNUSED would - * give a warning in a compilation where the parameter is indeed used, - * and not marking such a parameter would give a warning in a - * compilation where the parameter is unused. + * Depending on a configuration, all uses of such a thing may become + * #ifdef'ed away. Marking it with UNUSED would give a warning in a + * compilation where it is indeed used, and not marking it at all + * would give a warning in a compilation where it is unused. In such + * a case, MAYBE_UNUSED is the appropriate annotation to use. */ #define MAYBE_UNUSED __attribute__((__unused__)) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index d0fc7cfe60..3263245b03 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -262,8 +262,9 @@ For C programs: like "error: unused parameter 'foo' [-Werror=unused-parameter]", which indicates that a function ignores its argument. If the unused parameter can't be removed (e.g., because the function is used as a - callback and has to match a certain interface), you can annotate the - individual parameters with the UNUSED keyword, like "int foo UNUSED". + callback and has to match a certain interface), you can annotate + the individual parameters with the UNUSED (or MAYBE_UNUSED) + keyword, like "int foo UNUSED". - We try to support a wide range of C compilers to compile Git with, including old ones. As of Git v2.35.0 Git requires C99 (we check diff --git a/git-compat-util.h b/git-compat-util.h index 71b4d23f03..e4a306dd56 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -195,6 +195,19 @@ struct strbuf; #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 +/* + * UNUSED marks a function parameter that is always unused. It also + * can be used to annotate a function, a variable, or a type that is + * always unused. + * + * A callback interface may dictate that a function accepts a + * parameter at that position, but the implementation of the function + * may not need to use the parameter. In such a case, mark the parameter + * with UNUSED. + * + * When a parameter may be used or unused, depending on conditional + * compilation, consider using MAYBE_UNUSED instead. + */ #if GIT_GNUC_PREREQ(4, 5) #define UNUSED __attribute__((unused)) \ __attribute__((deprecated ("parameter declared as UNUSED"))) @@ -649,6 +662,17 @@ static inline int git_has_dir_sep(const char *path) #define RESULT_MUST_BE_USED #endif +/* + * MAYBE_UNUSED marks a function parameter that may be unused, but + * whose use is not an error. It also can be used to annotate a + * function, a variable, or a type that may be unused. + * + * Depending on a configuration, all uses of such a thing may become + * #ifdef'ed away. Marking it with UNUSED would give a warning in a + * compilation where it is indeed used, and not marking it at all + * would give a warning in a compilation where it is unused. In such + * a case, MAYBE_UNUSED is the appropriate annotation to use. + */ #define MAYBE_UNUSED __attribute__((__unused__)) #include "compat/bswap.h" From patchwork Thu Aug 29 18:27:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13783543 Received: from pb-smtp1.pobox.com (pb-smtp1.pobox.com [64.147.108.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F2A151B1D41 for ; Thu, 29 Aug 2024 18:27:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.70 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724956064; cv=none; b=SPbOa6hPjvDiF+eSDulTKEbVNCFbid3iX9HdsM6Bb8zD8UN333ucN21rbyZlpOY5WEG5mH1Xbhu9dXZhLJR4M2iJq5FU3Q7vscd/7dFu4sSVZRMej/WTx61k//qeVCdDKSZ77PvXzKNAbo1pAEkfBI3A3a9paO2+aNqtW1UqB7I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724956064; c=relaxed/simple; bh=HlG+WkLkWghzg5v9E/Fm4JxwpuymfGl5JvJqyybowes=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=HOYiAWcZQaXChJOQfWkzXS4338MEOY83etWPMUXH5GqgXbh7xltn2Swb4IIS6r8DOOrtXoNCn7odG98ShSyVxJZsxKMl966XHzB+Cxk25u82m6/th3gfJpWF856XCGS/BRJtpyztv+iNm6kutlEcdCqdRBowbyyYQSg1dth4tXc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=u97MsUtv; arc=none smtp.client-ip=64.147.108.70 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="u97MsUtv" Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 01D8E24362; Thu, 29 Aug 2024 14:27:42 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:in-reply-to:references:date:message-id:mime-version :content-type; s=sasl; bh=HlG+WkLkWghzg5v9E/Fm4JxwpuymfGl5JvJqyy bowes=; b=u97MsUtvbJJNPI2BN4wKLfD/uriH1dvIzzlw/P6SQ3UJ2JFOsc/nM1 3UnxPu8RZQ4RbmJW4j2N4FUwzvsNTGh8AAbKgz1djuC/9s6UIgLNITHgkuRnYYEb vC5+vgsX4PrNlN62gJbOF4jy8ja2iisxIDdolIz79i5wQURfkY0mI= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id EEE0A24361; Thu, 29 Aug 2024 14:27:41 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.94.240]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id 6588524360; Thu, 29 Aug 2024 14:27:40 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: Jeff King Cc: git@vger.kernel.org Subject: [PATCH 9/6] git-compat-util: guard definition of MAYBE_UNUSED with __GNUC__ In-Reply-To: (Junio C. Hamano's message of "Thu, 29 Aug 2024 11:18:06 -0700") References: <20240829040215.GA4054823@coredump.intra.peff.net> <20240829175215.GA415423@coredump.intra.peff.net> Date: Thu, 29 Aug 2024 11:27:39 -0700 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: 5FDB12E0-6634-11EF-8174-2BAEEB2EC81B-77302942!pb-smtp1.pobox.com Just like we only define UNUSED macro when __GNUC__ is defined, and fall back to an empty definition otherwise, we should do the same for MAYBE_UNUSED. Signed-off-by: Junio C Hamano --- * Before I forget that we have discussed this, just as a documentation (read: this is not a patch to be applied). I think this only matters when a compiler satisfies all three traits: - does not define __GNUC__ - does have its own __attribute__() macro - barfs on __attribute__((__unused__)) Otherwise we will define __attribute__(x) away to empty to cause no harm. Since we have survived without complaints without such a guard for quite some time, it may be a sign that no compiler that knows __attribute__() that people ever tried to compile Git with barfs with __attribute__((__unused__)). I dunno. git-compat-util.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index e4a306dd56..74ed581b5d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -673,7 +673,11 @@ static inline int git_has_dir_sep(const char *path) * would give a warning in a compilation where it is unused. In such * a case, MAYBE_UNUSED is the appropriate annotation to use. */ +#ifdef __GNUC__ #define MAYBE_UNUSED __attribute__((__unused__)) +#else +#define MAYBE_UNUSED +#endif #include "compat/bswap.h"