public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] gccrs: avoid printing to stderr in selftest::rust_flatten_list
@ 2022-12-16 17:01 David Malcolm
  2023-01-02 12:47 ` Arthur Cohen
  0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2022-12-16 17:01 UTC (permalink / raw)
  To: gcc-patches, Arthur Cohen; +Cc: David Malcolm

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/rust/ChangeLog:
	* resolve/rust-ast-resolve-item.cc (selftest::rust_flatten_list):
	Remove output to stderr.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/rust/resolve/rust-ast-resolve-item.cc | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc b/gcc/rust/resolve/rust-ast-resolve-item.cc
index 0c38f28d530..1276e845acc 100644
--- a/gcc/rust/resolve/rust-ast-resolve-item.cc
+++ b/gcc/rust/resolve/rust-ast-resolve-item.cc
@@ -1202,9 +1202,6 @@ rust_flatten_list (void)
   auto paths = std::vector<Rust::AST::SimplePath> ();
   Rust::Resolver::flatten_list (list, paths);
 
-  for (auto &path : paths)
-    fprintf (stderr, "%s\n", path.as_string ().c_str ());
-
   ASSERT_TRUE (!paths.empty ());
   ASSERT_EQ (paths.size (), 2);
   ASSERT_EQ (paths[0].get_segments ()[0].as_string (), "foo");
-- 
2.26.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gccrs: avoid printing to stderr in selftest::rust_flatten_list
  2022-12-16 17:01 [PATCH] gccrs: avoid printing to stderr in selftest::rust_flatten_list David Malcolm
@ 2023-01-02 12:47 ` Arthur Cohen
  2023-01-04 19:28   ` David Malcolm
  0 siblings, 1 reply; 6+ messages in thread
From: Arthur Cohen @ 2023-01-02 12:47 UTC (permalink / raw)
  To: David Malcolm, gcc-patches


[-- Attachment #1.1.1: Type: text/plain, Size: 1483 bytes --]

Hi David,

Sorry for the delayed reply!

On 12/16/22 18:01, David Malcolm wrote:
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> 
> OK for trunk?
> 
> gcc/rust/ChangeLog:
> 	* resolve/rust-ast-resolve-item.cc (selftest::rust_flatten_list):
> 	Remove output to stderr.
> 
> Signed-off-by: David Malcolm <dmalcolm@redhat.com>
> ---
>   gcc/rust/resolve/rust-ast-resolve-item.cc | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc b/gcc/rust/resolve/rust-ast-resolve-item.cc
> index 0c38f28d530..1276e845acc 100644
> --- a/gcc/rust/resolve/rust-ast-resolve-item.cc
> +++ b/gcc/rust/resolve/rust-ast-resolve-item.cc
> @@ -1202,9 +1202,6 @@ rust_flatten_list (void)
>     auto paths = std::vector<Rust::AST::SimplePath> ();
>     Rust::Resolver::flatten_list (list, paths);
>   
> -  for (auto &path : paths)
> -    fprintf (stderr, "%s\n", path.as_string ().c_str ());
> -
>     ASSERT_TRUE (!paths.empty ());
>     ASSERT_EQ (paths.size (), 2);
>     ASSERT_EQ (paths[0].get_segments ()[0].as_string (), "foo");

Looks good to me. OK for trunk :)

Thanks for taking the time!

All the best,

-- 
Arthur Cohen <arthur.cohen@embecosm.com>

Toolchain Engineer

Embecosm GmbH

Geschäftsführer: Jeremy Bennett
Niederlassung: Nürnberg
Handelsregister: HR-B 36368
www.embecosm.de

Fürther Str. 27
90429 Nürnberg


Tel.: 091 - 128 707 040
Fax: 091 - 128 707 077

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3195 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gccrs: avoid printing to stderr in selftest::rust_flatten_list
  2023-01-02 12:47 ` Arthur Cohen
@ 2023-01-04 19:28   ` David Malcolm
  2023-01-05 14:44     ` Arthur Cohen
  0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2023-01-04 19:28 UTC (permalink / raw)
  To: Arthur Cohen, gcc-patches

On Mon, 2023-01-02 at 13:47 +0100, Arthur Cohen wrote:
> Hi David,
> 
> Sorry for the delayed reply!
> 
> On 12/16/22 18:01, David Malcolm wrote:
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> > 
> > OK for trunk?
> > 
> > gcc/rust/ChangeLog:
> >         * resolve/rust-ast-resolve-item.cc
> > (selftest::rust_flatten_list):
> >         Remove output to stderr.

For reference, the stderr spewage was:

foo::bar::baz
foo::bar::bul


> > 
> > Signed-off-by: David Malcolm <dmalcolm@redhat.com>
> > ---
> >   gcc/rust/resolve/rust-ast-resolve-item.cc | 3 ---
> >   1 file changed, 3 deletions(-)
> > 
> > diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc
> > b/gcc/rust/resolve/rust-ast-resolve-item.cc
> > index 0c38f28d530..1276e845acc 100644
> > --- a/gcc/rust/resolve/rust-ast-resolve-item.cc
> > +++ b/gcc/rust/resolve/rust-ast-resolve-item.cc
> > @@ -1202,9 +1202,6 @@ rust_flatten_list (void)
> >     auto paths = std::vector<Rust::AST::SimplePath> ();
> >     Rust::Resolver::flatten_list (list, paths);
> >   
> > -  for (auto &path : paths)
> > -    fprintf (stderr, "%s\n", path.as_string ().c_str ());
> > -
> >     ASSERT_TRUE (!paths.empty ());
> >     ASSERT_EQ (paths.size (), 2);
> >     ASSERT_EQ (paths[0].get_segments ()[0].as_string (), "foo");
> 
> Looks good to me. OK for trunk :)
> 
> Thanks for taking the time!

I was about to push this and
  https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608645.html
to trunk (after retesting against all the changes since before my
break), but you mentioned today on IRC something about merger issues:

<cohenarthur> ibuclaw: I'm looking at the remaining issues for updating
GCC's master with our most recent gccrs commits. I've come to the
conclusion that it would be easier for me to upstream your target
changes while I'm upstreaming/submitting all of the missing commits
<cohenarthur> would that suit you?
<cohenarthur> it would just be me sending in your commits, but I
wouldn't be author on them or anything of course
<cohenarthur> this would enable us to time them properly within the
rest of the commits, so there'd be no conflicts or anything of the sort
<cohenarthur> dmalcolm: same question to you, actually :)

Can I go ahead and push my two commits to trunk, or do you want to do
it?  (and if so, do you want them e.g. as PRs against your github
branch?)

Dave


> 
> All the best,


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gccrs: avoid printing to stderr in selftest::rust_flatten_list
  2023-01-04 19:28   ` David Malcolm
@ 2023-01-05 14:44     ` Arthur Cohen
  2023-01-05 15:36       ` David Malcolm
  0 siblings, 1 reply; 6+ messages in thread
From: Arthur Cohen @ 2023-01-05 14:44 UTC (permalink / raw)
  To: David Malcolm, gcc-patches; +Cc: Iain Buclaw


[-- Attachment #1.1.1: Type: text/plain, Size: 4279 bytes --]

Hi David,

On 1/4/23 20:28, David Malcolm wrote:
> On Mon, 2023-01-02 at 13:47 +0100, Arthur Cohen wrote:
>> Hi David,
>>
>> Sorry for the delayed reply!
>>
>> On 12/16/22 18:01, David Malcolm wrote:
>>> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>>>
>>> OK for trunk?
>>>
>>> gcc/rust/ChangeLog:
>>>          * resolve/rust-ast-resolve-item.cc
>>> (selftest::rust_flatten_list):
>>>          Remove output to stderr.
> 
> For reference, the stderr spewage was:
> 
> foo::bar::baz
> foo::bar::bul
> >
>>>
>>> Signed-off-by: David Malcolm <dmalcolm@redhat.com>
>>> ---
>>>    gcc/rust/resolve/rust-ast-resolve-item.cc | 3 ---
>>>    1 file changed, 3 deletions(-)
>>>
>>> diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc
>>> b/gcc/rust/resolve/rust-ast-resolve-item.cc
>>> index 0c38f28d530..1276e845acc 100644
>>> --- a/gcc/rust/resolve/rust-ast-resolve-item.cc
>>> +++ b/gcc/rust/resolve/rust-ast-resolve-item.cc
>>> @@ -1202,9 +1202,6 @@ rust_flatten_list (void)
>>>      auto paths = std::vector<Rust::AST::SimplePath> ();
>>>      Rust::Resolver::flatten_list (list, paths);
>>>    
>>> -  for (auto &path : paths)
>>> -    fprintf (stderr, "%s\n", path.as_string ().c_str ());
>>> -
>>>      ASSERT_TRUE (!paths.empty ());
>>>      ASSERT_EQ (paths.size (), 2);
>>>      ASSERT_EQ (paths[0].get_segments ()[0].as_string (), "foo");
>>
>> Looks good to me. OK for trunk :)
>>
>> Thanks for taking the time!
> 
> I was about to push this and
>    https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608645.html
> to trunk (after retesting against all the changes since before my
> break), but you mentioned today on IRC something about merger issues:
> 
> <cohenarthur> ibuclaw: I'm looking at the remaining issues for updating
> GCC's master with our most recent gccrs commits. I've come to the
> conclusion that it would be easier for me to upstream your target
> changes while I'm upstreaming/submitting all of the missing commits
> <cohenarthur> would that suit you?
> <cohenarthur> it would just be me sending in your commits, but I
> wouldn't be author on them or anything of course
> <cohenarthur> this would enable us to time them properly within the
> rest of the commits, so there'd be no conflicts or anything of the sort
> <cohenarthur> dmalcolm: same question to you, actually :)

Sorry for the confusion, and for disappearing from IRC before you got a 
chance to answer! In those messages, I was talking about the 
`error_meta` PR you had submitted to us on Github. Since we are in the 
process of updating upstream with the current state of our dev branch, 
we have to figure out what to do with these commits that are already 
present in our dev branch but not upstreamed yet. Since you and Iain 
have pushed commits to our dev branch, we are wondering whether you'd 
like us to upstream them during this updating process or if you'd like 
to do so on your own.

Regarding the two new patches that you've submitted here and that aren't 
upstreamed or merged in our dev branch yet, it's a bit different. You 
can either go ahead and push them to trunk if that's what you'd like, 
and I'm assuming that when we merge upstream and our dev branch this 
won't cause any conflict. And if these two patches do, they are easy 
enough that we can fix them by hand.

If you'd like, you can also submit a PR instead, and we'll upstream them 
when updating the rest of the frontend upstream, similarly to your 
`error_meta` patches.

Last option, I can also take care of them and merge them directly in our 
dev branch, and we'll upstream them with the rest of the frontend. This 
way you don't have to deal with submitting a PR and so on.

> Can I go ahead and push my two commits to trunk, or do you want to do
> it?  (and if so, do you want them e.g. as PRs against your github
> branch?)
> 
> Dave
> 
> 
>>
>> All the best,
> 
Sorry about all of this. We are working hard on updating upstream so 
that there's no such problems anymore. We'll get that done as soon as 
possible, but winter break put quite the wrench in our plans :)

Thank you for your understanding!

All the best,

Arthur


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3195 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gccrs: avoid printing to stderr in selftest::rust_flatten_list
  2023-01-05 14:44     ` Arthur Cohen
@ 2023-01-05 15:36       ` David Malcolm
  2023-01-05 16:36         ` Arthur Cohen
  0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2023-01-05 15:36 UTC (permalink / raw)
  To: Arthur Cohen, gcc-patches; +Cc: Iain Buclaw

On Thu, 2023-01-05 at 15:44 +0100, Arthur Cohen wrote:
> Hi David,
> 
> On 1/4/23 20:28, David Malcolm wrote:
> > On Mon, 2023-01-02 at 13:47 +0100, Arthur Cohen wrote:
> > > Hi David,
> > > 
> > > Sorry for the delayed reply!
> > > 
> > > On 12/16/22 18:01, David Malcolm wrote:
> > > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> > > > 
> > > > OK for trunk?
> > > > 
> > > > gcc/rust/ChangeLog:
> > > >          * resolve/rust-ast-resolve-item.cc
> > > > (selftest::rust_flatten_list):
> > > >          Remove output to stderr.
> > 
> > For reference, the stderr spewage was:
> > 
> > foo::bar::baz
> > foo::bar::bul
> > > 
> > > > 
> > > > Signed-off-by: David Malcolm <dmalcolm@redhat.com>
> > > > ---
> > > >    gcc/rust/resolve/rust-ast-resolve-item.cc | 3 ---
> > > >    1 file changed, 3 deletions(-)
> > > > 
> > > > diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc
> > > > b/gcc/rust/resolve/rust-ast-resolve-item.cc
> > > > index 0c38f28d530..1276e845acc 100644
> > > > --- a/gcc/rust/resolve/rust-ast-resolve-item.cc
> > > > +++ b/gcc/rust/resolve/rust-ast-resolve-item.cc
> > > > @@ -1202,9 +1202,6 @@ rust_flatten_list (void)
> > > >      auto paths = std::vector<Rust::AST::SimplePath> ();
> > > >      Rust::Resolver::flatten_list (list, paths);
> > > >    
> > > > -  for (auto &path : paths)
> > > > -    fprintf (stderr, "%s\n", path.as_string ().c_str ());
> > > > -
> > > >      ASSERT_TRUE (!paths.empty ());
> > > >      ASSERT_EQ (paths.size (), 2);
> > > >      ASSERT_EQ (paths[0].get_segments ()[0].as_string (),
> > > > "foo");
> > > 
> > > Looks good to me. OK for trunk :)
> > > 
> > > Thanks for taking the time!
> > 
> > I was about to push this and
> >   
> > https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608645.html
> > to trunk (after retesting against all the changes since before my
> > break), but you mentioned today on IRC something about merger
> > issues:
> > 
> > <cohenarthur> ibuclaw: I'm looking at the remaining issues for
> > updating
> > GCC's master with our most recent gccrs commits. I've come to the
> > conclusion that it would be easier for me to upstream your target
> > changes while I'm upstreaming/submitting all of the missing commits
> > <cohenarthur> would that suit you?
> > <cohenarthur> it would just be me sending in your commits, but I
> > wouldn't be author on them or anything of course
> > <cohenarthur> this would enable us to time them properly within the
> > rest of the commits, so there'd be no conflicts or anything of the
> > sort
> > <cohenarthur> dmalcolm: same question to you, actually :)
> 
> Sorry for the confusion, and for disappearing from IRC before you got
> a 
> chance to answer! In those messages, I was talking about the 
> `error_meta` PR you had submitted to us on Github. Since we are in
> the 
> process of updating upstream with the current state of our dev
> branch, 
> we have to figure out what to do with these commits that are already 
> present in our dev branch but not upstreamed yet. Since you and Iain 
> have pushed commits to our dev branch, we are wondering whether you'd
> like us to upstream them during this updating process or if you'd
> like 
> to do so on your own.

For reference, presumably this is:
  https://github.com/Rust-GCC/gccrs/pull/1408
which was kind of an experimenal proof-of-concept on my part.

As far as I can tell, that PR added some relatively trivial new stuff
to gcc/diagnostic*, and otherwise just touched the gcc/rust
subdirectory, so I hope it's relatively trivial to merge.

I'd prefer to leave the responsibility for merging that work into trunk
to you.

> 
> Regarding the two new patches that you've submitted here and that
> aren't 
> upstreamed or merged in our dev branch yet, it's a bit different. You
> can either go ahead and push them to trunk if that's what you'd like,
> and I'm assuming that when we merge upstream and our dev branch this 
> won't cause any conflict. And if these two patches do, they are easy 
> enough that we can fix them by hand.

Thanks.  I've gone ahead and pushed the two simple patches to trunk as:
  r13-5031-gb0edfa0ef02c0f
and:
  r13-5032-gefce0caf2d75df

Dave


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gccrs: avoid printing to stderr in selftest::rust_flatten_list
  2023-01-05 15:36       ` David Malcolm
@ 2023-01-05 16:36         ` Arthur Cohen
  0 siblings, 0 replies; 6+ messages in thread
From: Arthur Cohen @ 2023-01-05 16:36 UTC (permalink / raw)
  To: David Malcolm, gcc-patches; +Cc: Iain Buclaw


[-- Attachment #1.1.1: Type: text/plain, Size: 4421 bytes --]



On 1/5/23 16:36, David Malcolm wrote:
> On Thu, 2023-01-05 at 15:44 +0100, Arthur Cohen wrote:
>> Hi David,
>>
>> On 1/4/23 20:28, David Malcolm wrote:
>>> On Mon, 2023-01-02 at 13:47 +0100, Arthur Cohen wrote:
>>>> Hi David,
>>>>
>>>> Sorry for the delayed reply!
>>>>
>>>> On 12/16/22 18:01, David Malcolm wrote:
>>>>> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>>>>>
>>>>> OK for trunk?
>>>>>
>>>>> gcc/rust/ChangeLog:
>>>>>           * resolve/rust-ast-resolve-item.cc
>>>>> (selftest::rust_flatten_list):
>>>>>           Remove output to stderr.
>>>
>>> For reference, the stderr spewage was:
>>>
>>> foo::bar::baz
>>> foo::bar::bul
>>>>
>>>>>
>>>>> Signed-off-by: David Malcolm <dmalcolm@redhat.com>
>>>>> ---
>>>>>     gcc/rust/resolve/rust-ast-resolve-item.cc | 3 ---
>>>>>     1 file changed, 3 deletions(-)
>>>>>
>>>>> diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc
>>>>> b/gcc/rust/resolve/rust-ast-resolve-item.cc
>>>>> index 0c38f28d530..1276e845acc 100644
>>>>> --- a/gcc/rust/resolve/rust-ast-resolve-item.cc
>>>>> +++ b/gcc/rust/resolve/rust-ast-resolve-item.cc
>>>>> @@ -1202,9 +1202,6 @@ rust_flatten_list (void)
>>>>>       auto paths = std::vector<Rust::AST::SimplePath> ();
>>>>>       Rust::Resolver::flatten_list (list, paths);
>>>>>     
>>>>> -  for (auto &path : paths)
>>>>> -    fprintf (stderr, "%s\n", path.as_string ().c_str ());
>>>>> -
>>>>>       ASSERT_TRUE (!paths.empty ());
>>>>>       ASSERT_EQ (paths.size (), 2);
>>>>>       ASSERT_EQ (paths[0].get_segments ()[0].as_string (),
>>>>> "foo");
>>>>
>>>> Looks good to me. OK for trunk :)
>>>>
>>>> Thanks for taking the time!
>>>
>>> I was about to push this and
>>>    
>>> https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608645.html
>>> to trunk (after retesting against all the changes since before my
>>> break), but you mentioned today on IRC something about merger
>>> issues:
>>>
>>> <cohenarthur> ibuclaw: I'm looking at the remaining issues for
>>> updating
>>> GCC's master with our most recent gccrs commits. I've come to the
>>> conclusion that it would be easier for me to upstream your target
>>> changes while I'm upstreaming/submitting all of the missing commits
>>> <cohenarthur> would that suit you?
>>> <cohenarthur> it would just be me sending in your commits, but I
>>> wouldn't be author on them or anything of course
>>> <cohenarthur> this would enable us to time them properly within the
>>> rest of the commits, so there'd be no conflicts or anything of the
>>> sort
>>> <cohenarthur> dmalcolm: same question to you, actually :)
>>
>> Sorry for the confusion, and for disappearing from IRC before you got
>> a
>> chance to answer! In those messages, I was talking about the
>> `error_meta` PR you had submitted to us on Github. Since we are in
>> the
>> process of updating upstream with the current state of our dev
>> branch,
>> we have to figure out what to do with these commits that are already
>> present in our dev branch but not upstreamed yet. Since you and Iain
>> have pushed commits to our dev branch, we are wondering whether you'd
>> like us to upstream them during this updating process or if you'd
>> like
>> to do so on your own.
> 
> For reference, presumably this is:
>    https://github.com/Rust-GCC/gccrs/pull/1408
> which was kind of an experimenal proof-of-concept on my part.
> 
> As far as I can tell, that PR added some relatively trivial new stuff
> to gcc/diagnostic*, and otherwise just touched the gcc/rust
> subdirectory, so I hope it's relatively trivial to merge.
> 
> I'd prefer to leave the responsibility for merging that work into trunk
> to you.

Sounds good!

> 
>>
>> Regarding the two new patches that you've submitted here and that
>> aren't
>> upstreamed or merged in our dev branch yet, it's a bit different. You
>> can either go ahead and push them to trunk if that's what you'd like,
>> and I'm assuming that when we merge upstream and our dev branch this
>> won't cause any conflict. And if these two patches do, they are easy
>> enough that we can fix them by hand.
> 
> Thanks.  I've gone ahead and pushed the two simple patches to trunk as:
>    r13-5031-gb0edfa0ef02c0f
> and:
>    r13-5032-gefce0caf2d75df
> 
> Dave
> 
Great :) Thanks a lot!

Arthur

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3195 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-05 16:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-16 17:01 [PATCH] gccrs: avoid printing to stderr in selftest::rust_flatten_list David Malcolm
2023-01-02 12:47 ` Arthur Cohen
2023-01-04 19:28   ` David Malcolm
2023-01-05 14:44     ` Arthur Cohen
2023-01-05 15:36       ` David Malcolm
2023-01-05 16:36         ` Arthur Cohen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).