[PATCH 0/4] P1689 followup fixes

[PATCH 0/4] P1689 followup fixes

[Ben Boeckel]

Hi,

Here are some minor fixes to documentation, formatting, and styling to the
P1689R5 support through the `-fdeps-*` flags.

Thanks,

--Ben

Ben Boeckel (4):
  libcpp/mkdeps: fix indentation
  libcpp/init: remove unnecessary `struct` keyword
  gcc/c-family/c-opts: fix quoting for `-fdeps-format=` error message
  gcc/c-family/c.opt: clarify `-fdeps-*` flag documentation

 gcc/c-family/c-opts.cc |  2 +-
 gcc/c-family/c.opt     |  6 +++---
 libcpp/init.cc         |  2 +-
 libcpp/mkdeps.cc       | 11 ++++++-----
 4 files changed, 11 insertions(+), 10 deletions(-)


base-commit: bba118db3f63cb1e3953a014aa3ac2ad89908950
-- 
2.44.0

[PATCH 1/4] libcpp/mkdeps: fix indentation

[Ben Boeckel]

Fixes: 024f135a1e9 (p1689r5: initial support, 2023-09-01)

Reported-by: Roland Illig <roland.illig@gmx.de>

libcpp/

	* mkdeps.cc (fdeps_add_target): Fix indentation.

Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
---
 libcpp/mkdeps.cc | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/libcpp/mkdeps.cc b/libcpp/mkdeps.cc
index 4cf0cf09178..8762ead4c34 100644
--- a/libcpp/mkdeps.cc
+++ b/libcpp/mkdeps.cc
@@ -307,11 +307,12 @@ fdeps_add_target (struct mkdeps *d, const char *o, bool is_primary)
 {
   o = apply_vpath (d, o);
   if (is_primary)
-  {
-    if (d->primary_output)
-      d->fdeps_targets.push (d->primary_output);
-    d->primary_output = xstrdup (o);
-  } else
+    {
+      if (d->primary_output)
+	d->fdeps_targets.push (d->primary_output);
+      d->primary_output = xstrdup (o);
+    }
+  else
     d->fdeps_targets.push (xstrdup (o));
 }
 
-- 
2.44.0

[PATCH 2/4] libcpp/init: remove unnecessary `struct` keyword

[Ben Boeckel]

The initial P1689 patches were written in 2019 and ended up having code
move around over time ended up introducing a `struct` keyword to the
implementation of `cpp_finish`. Remove it to match the rest of the file
and its declaration in the header.

Fixes: 024f135a1e9 (p1689r5: initial support, 2023-09-01)

Reported-by: Roland Illig <roland.illig@gmx.de>

libcpp/

	* init.cc (cpp_finish): Remove unnecessary `struct` keyword.

Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
---
 libcpp/init.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcpp/init.cc b/libcpp/init.cc
index 54fc9236d38..cbd22249b04 100644
--- a/libcpp/init.cc
+++ b/libcpp/init.cc
@@ -862,7 +862,7 @@ read_original_directory (cpp_reader *pfile)
    Maybe it should also reset state, such that you could call
    cpp_start_read with a new filename to restart processing.  */
 void
-cpp_finish (struct cpp_reader *pfile, FILE *deps_stream, FILE *fdeps_stream)
+cpp_finish (cpp_reader *pfile, FILE *deps_stream, FILE *fdeps_stream)
 {
   /* Warn about unused macros before popping the final buffer.  */
   if (CPP_OPTION (pfile, warn_unused_macros))
-- 
2.44.0

[PATCH 3/4] gcc/c-family/c-opts: fix quoting for `-fdeps-format=` error message

[Ben Boeckel]

Fixes: 024f135a1e9 (p1689r5: initial support, 2023-09-01)

Reported-by: Roland Illig <roland.illig@gmx.de>

gcc/c-family/

	* c-opts.cc (c_common_handle_option): Fix quoting in
	`-fdeps-format=` unrecognized parameter error message.

Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
---
 gcc/c-family/c-opts.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index be3058dca63..4a164ad0c0b 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -370,7 +370,7 @@ c_common_handle_option (size_t scode, const char *arg, HOST_WIDE_INT value,
       if (!strcmp (arg, "p1689r5"))
 	cpp_opts->deps.fdeps_format = FDEPS_FMT_P1689R5;
       else
-	error ("%<-fdeps-format=%> unknown format %<%s%>", arg);
+	error ("%<-fdeps-format=%> unknown format %q", arg);
       break;
 
     case OPT_fdeps_file_:
-- 
2.44.0

[PATCH 4/4] gcc/c-family/c.opt: clarify `-fdeps-*` flag documentation

[Ben Boeckel]

Move the only supported value (as of today) to the flag name itself.
Also reword to clarify that the `-fdeps-file=` file will be written to.

Fixes: 024f135a1e9 (p1689r5: initial support, 2023-09-01)

Reported-by: Roland Illig <roland.illig@gmx.de>

gcc/c-family/

	* c.opt: Clarify `-fdeps-*` documentation.

Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
---
 gcc/c-family/c.opt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 56cccf2a67b..fa82eebb518 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -256,13 +256,13 @@ MT
 C ObjC C++ ObjC++ Joined Separate MissingArgError(missing makefile target after %qs)
 -MT <target>	Add a target that does not require quoting.
 
-fdeps-format=
+fdeps-format=p1689r5
 C ObjC C++ ObjC++ NoDriverArg Joined MissingArgError(missing format after %qs)
-Structured format for output dependency information.  Supported (\"p1689r5\").
+Structured format for output dependency information.
 
 fdeps-file=
 C ObjC C++ ObjC++ NoDriverArg Joined MissingArgError(missing output path after %qs)
-File for output dependency information.
+File to write dependency information to.
 
 fdeps-target=
 C ObjC C++ ObjC++ NoDriverArg Joined MissingArgError(missing path after %qs)
-- 
2.44.0

Re: [PATCH 3/4] gcc/c-family/c-opts: fix quoting for `-fdeps-format=` error message

[Joseph Myers]

On Sat, 4 May 2024, Ben Boeckel wrote:

> diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
> index be3058dca63..4a164ad0c0b 100644
> --- a/gcc/c-family/c-opts.cc
> +++ b/gcc/c-family/c-opts.cc
> @@ -370,7 +370,7 @@ c_common_handle_option (size_t scode, const char *arg, HOST_WIDE_INT value,
>        if (!strcmp (arg, "p1689r5"))
>  	cpp_opts->deps.fdeps_format = FDEPS_FMT_P1689R5;
>        else
> -	error ("%<-fdeps-format=%> unknown format %<%s%>", arg);
> +	error ("%<-fdeps-format=%> unknown format %q", arg);
>        break;

That can't be right.  The GCC %q is a modifier that needs to have an 
actual format specifier it modifies (so %qs - which produces the same 
output as %<%s%> - but not %q by itself).

-- 
Joseph S. Myers
josmyers@redhat.com

Re: [PATCH 3/4] gcc/c-family/c-opts: fix quoting for `-fdeps-format=` error message

[Ben Boeckel]

On Tue, May 07, 2024 at 21:15:09 +0000, Joseph Myers wrote:
> That can't be right.  The GCC %q is a modifier that needs to have an 
> actual format specifier it modifies (so %qs - which produces the same 
> output as %<%s%> - but not %q by itself).

Yes, I got CI results of failure and noticed that I had prepared the
patches on my laptop, but when I investigated, I had done additional
work on my desktop concurrently I had not pulled back (it builds GCC in
a…reasonable time comparatively) which did have the `%qs` change, but
I've not gotten around to running the test suite again (or reporting
back here). I have another patch revision in the works.

Thanks,

--Ben

Re: [PATCH 2/4] libcpp/init: remove unnecessary `struct` keyword

[Richard Biener]

On Sat, May 4, 2024 at 5:06 PM Ben Boeckel <ben.boeckel@kitware.com> wrote:
>
> The initial P1689 patches were written in 2019 and ended up having code
> move around over time ended up introducing a `struct` keyword to the
> implementation of `cpp_finish`. Remove it to match the rest of the file
> and its declaration in the header.
>
> Fixes: 024f135a1e9 (p1689r5: initial support, 2023-09-01)
>
> Reported-by: Roland Illig <roland.illig@gmx.de>

OK.

Thanks,
Richard.

> libcpp/
>
>         * init.cc (cpp_finish): Remove unnecessary `struct` keyword.
>
> Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
> ---
>  libcpp/init.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libcpp/init.cc b/libcpp/init.cc
> index 54fc9236d38..cbd22249b04 100644
> --- a/libcpp/init.cc
> +++ b/libcpp/init.cc
> @@ -862,7 +862,7 @@ read_original_directory (cpp_reader *pfile)
>     Maybe it should also reset state, such that you could call
>     cpp_start_read with a new filename to restart processing.  */
>  void
> -cpp_finish (struct cpp_reader *pfile, FILE *deps_stream, FILE *fdeps_stream)
> +cpp_finish (cpp_reader *pfile, FILE *deps_stream, FILE *fdeps_stream)
>  {
>    /* Warn about unused macros before popping the final buffer.  */
>    if (CPP_OPTION (pfile, warn_unused_macros))
> --
> 2.44.0
>