Received-SPF: pass (google.com: domain of linux-kernel+bounces-202872-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240605140453.614862-1-clement.legoffic@foss.st.com>
In-Reply-To: <20240605140453.614862-1-clement.legoffic@foss.st.com>
From: Ian Rogers <irogers@google.com>
Date: Wed, 5 Jun 2024 08:55:51 -0700
Message-ID: <CAP-5=fUW6UWw=0iRe+r4JxrxZ3Yqnpd0_ZhrQx0SidfRDf5YgA@mail.gmail.com>
Subject: Re: [RFC PATCH] perf: parse-events: Fix compilation error while
 defining DEBUG_PARSER
To: =?UTF-8?Q?Cl=C3=A9ment_Le_Goffic?= <clement.legoffic@foss.st.com>
Cc: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, 
	Arnaldo Carvalho de Melo <acme@kernel.org>, Namhyung Kim <namhyung@kernel.org>, 
	Mark Rutland <mark.rutland@arm.com>, 
	Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, 
	Adrian Hunter <adrian.hunter@intel.com>, "Liang, Kan" <kan.liang@linux.intel.com>, 
	John Garry <john.g.garry@oracle.com>, James Clark <james.clark@arm.com>, 
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 5, 2024 at 7:06=E2=80=AFAM Cl=C3=A9ment Le Goffic
<clement.legoffic@foss.st.com> wrote:
>
> Compiling perf tool with 'DEBUG_PARSER=3D1' leads to errors:
>
> $> make -C tools/perf PARSER_DEBUG=3D1 NO_LIBTRACEEVENT=3D1
> ...
>   CC      util/expr-flex.o
>   CC      util/expr.o
> util/parse-events.c:33:12: error: redundant redeclaration of =E2=80=98par=
se_events_debug=E2=80=99 [-Werror=3Dredundant-decls]
>    33 | extern int parse_events_debug;
>       |            ^~~~~~~~~~~~~~~~~~
> In file included from util/parse-events.c:18:
> util/parse-events-bison.h:43:12: note: previous declaration of =E2=80=98p=
arse_events_debug=E2=80=99 with type =E2=80=98int=E2=80=99
>    43 | extern int parse_events_debug;
>       |            ^~~~~~~~~~~~~~~~~~
> util/expr.c:27:12: error: redundant redeclaration of =E2=80=98expr_debug=
=E2=80=99 [-Werror=3Dredundant-decls]
>    27 | extern int expr_debug;
>       |            ^~~~~~~~~~
> In file included from util/expr.c:11:
> util/expr-bison.h:43:12: note: previous declaration of =E2=80=98expr_debu=
g=E2=80=99 with type =E2=80=98int=E2=80=99
>    43 | extern int expr_debug;
>       |            ^~~~~~~~~~
> cc-1: all warnings being treated as errors
>
> Remove extern declaration from the parse-envents.c file as there is a
> conflict with the ones generated using bison and yacc tools from the file
> parse-events.[ly].

I encounter the same problem and routinely just do a local change to
comment out the declaration. The definition comes from the original
patch adding debug support, Commit 82ba1f2f6148 ("perf tools: Add
support for displaying event parser debug info") and so I believe
older flex/yacc tools needed the declaration. I think the right thing
to do is to remove them.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> Signed-off-by: Cl=C3=A9ment Le Goffic <clement.legoffic@foss.st.com>
> ---
> I don't know how bison and yacc work together.
> To me if those symbols are already generated (I don't know how), we
> should get rid of them in the source file `parse-events.c`.
> For my knowledge, if someone has the explanation of how they are
> generated, I am interested as I didn't find any link to some part of
> parse-events.[yl] files.
>
> To: Peter Zijlstra <peterz@infradead.org>
> To: Ingo Molnar <mingo@redhat.com>
> To: Arnaldo Carvalho de Melo <acme@kernel.org>
> To: Namhyung Kim <namhyung@kernel.org>
> To: Mark Rutland <mark.rutland@arm.com>
> To: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> To: Jiri Olsa <jolsa@kernel.org>
> To: Ian Rogers <irogers@google.com>
> To: Adrian Hunter <adrian.hunter@intel.com>
> To: "Liang, Kan" <kan.liang@linux.intel.com>
> To: John Garry <john.g.garry@oracle.com>
> To: James Clark <james.clark@arm.com>
> Cc: linux-perf-users@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  tools/perf/util/expr.c         | 4 ----
>  tools/perf/util/parse-events.c | 3 ---
>  2 files changed, 7 deletions(-)
>
> diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
> index b8875aac8f87..b2536a59c44e 100644
> --- a/tools/perf/util/expr.c
> +++ b/tools/perf/util/expr.c
> @@ -25,10 +25,6 @@
>  #include <math.h>
>  #include "pmu.h"
>
> -#ifdef PARSER_DEBUG
> -extern int expr_debug;
> -#endif
> -
>  struct expr_id_data {
>         union {
>                 struct {
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-event=
s.c
> index 6ed0f9c5581d..8d5fb05f20c2 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -31,9 +31,6 @@
>
>  #define MAX_NAME_LEN 100
>
> -#ifdef PARSER_DEBUG
> -extern int parse_events_debug;
> -#endif
>  static int get_config_terms(const struct parse_events_terms *head_config=
,
>                             struct list_head *head_terms);
>  static int parse_events_terms__copy(const struct parse_events_terms *src=
,
>
> base-commit: c3f38fa61af77b49866b006939479069cd451173
> --
> 2.34.1
>