Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2019-12-12 Thread nine . fierce . ballads
Coming back to this after a long time... I intend to abandon the pointer/reference changes and submit a new review covering just the second point: "Call find_top_context () rather than get_global_context () where the caller uses the result as a mere Context." Those interested in the

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-29 Thread dak
On 2018/04/25 23:05:08, Dan Eble wrote: On 2018/04/25 10:44:22, dak wrote: > It still has the disadvantage of adding a new class as baggage, making it harder > for newcomers interpreting code. In particular the necessity of overloading *, > &, and -> operators raises the amount of knowledge

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-25 Thread nine . fierce . ballads
On 2018/04/25 10:44:22, dak wrote: Please note that all of your proposals so far do _not_ provide the nullness information that you value in any manner: previously undefined behavior would continue to be undefined behavior and would just look differently. Yes, but what I believe you are

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-25 Thread dak
On 2018/04/23 23:55:44, Dan Eble wrote: David, would you accept a template class wrapping a non-null pointer, perhaps non_null_ptr? Conversion to a raw pointer would be implicit, but construction from a raw pointer would be explicit. The person constructing such a pointer is responsible

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-23 Thread nine . fierce . ballads
David, would you accept a template class wrapping a non-null pointer, perhaps non_null_ptr? Conversion to a raw pointer would be implicit, but construction from a raw pointer would be explicit. The person constructing such a pointer is responsible for ensuring that it is not null. The person

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-22 Thread dak
On 2018/04/22 13:56:01, Dan Eble wrote: On 2018/04/22 13:37:50, dak wrote: > So again I don't see what problem you are trying to solve. 1. find_something(...)->blahblah() I have no idea whether that will call blahblah() with a valid instance of something. C++ does not allow "this" to be

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-22 Thread nine . fierce . ballads
On 2018/04/22 13:37:50, dak wrote: So again I don't see what problem you are trying to solve. 1. find_something(...)->blahblah() I have no idea whether that will call blahblah() with a valid instance of something. 2. find_something(...).blahblah() I know will call blahblah() with a valid

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-22 Thread dak
https://codereview.appspot.com/341150043/diff/1/lily/context.cc File lily/context.cc (right): https://codereview.appspot.com/341150043/diff/1/lily/context.cc#newcode723 lily/context.cc:723: find_top_context (Context ) On 2018/04/22 13:19:45, Dan Eble wrote: On 2018/04/21 20:58:32, dak wrote: >

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-22 Thread nine . fierce . ballads
On 2018/04/22 13:19:46, Dan Eble wrote: of calls in some scope: if (Context *c = find_whatever(...)) { a(*p); b(*p); c(*p); }. of course I meant *c for *p https://codereview.appspot.com/341150043/ ___ lilypond-devel mailing list

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-22 Thread nine . fierce . ballads
https://codereview.appspot.com/341150043/diff/1/lily/context.cc File lily/context.cc (right): https://codereview.appspot.com/341150043/diff/1/lily/context.cc#newcode723 lily/context.cc:723: find_top_context (Context ) On 2018/04/21 20:58:32, dak wrote: And it was simpler to understand and

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-22 Thread nine . fierce . ballads
On 2018/04/22 01:18:22, Carl wrote: It seems like if there is a problem that this solves, there should be a regression test that shows the problem and hence why this patch is needed. The problems this solves are semantic. I should have classified this as "maintainability" in the ticket;

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-21 Thread Carl . D . Sorensen
It seems like if there is a problem that this solves, there should be a regression test that shows the problem and hence why this patch is needed. https://codereview.appspot.com/341150043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org

Re: Issue 5310: find_top_context () maintenance (issue 341150043 by nine.fierce.ball...@gmail.com)

2018-04-21 Thread dak
https://codereview.appspot.com/341150043/diff/1/lily/context.cc File lily/context.cc (right): https://codereview.appspot.com/341150043/diff/1/lily/context.cc#newcode723 lily/context.cc:723: find_top_context (Context ) What problem are you trying to fix here? find_top_context worked given a