Re: logging an optional input location

[Carl Sorensen]

On 5/12/18, 1:49 PM, "lilypond-devel on behalf of Dan Eble" <address@hidden on 
behalf of address@hidden> wrote:

    There’s this repetition in context.cc:
    
      if (origin)
        {
          origin->warning (_f ("cannot find or create `%s' called `%s'",
                               ly_symbol2string (n).c_str (), id));
        }
      else
        {
          warning (_f ("cannot find or create `%s' called `%s'",
                       ly_symbol2string (n).c_str (), id));
        }
    
    I put up with this when it was just one instance, but it’s starting to 
multiply in my work in progress.
    
    I see that ::warning already accepts a location string as an optional 
second parameter, so I am thinking of using that rather than creating yet 
another interface.  I am thinking of defining a helper function 
message_location(const Input*) to be used like this:
    
      warning (_f (. . .), message_location (origin));
    
    Are there any strong objections to this?  If so, what would you prefer?

It seems like a good idea to eliminate all of these duplications.

Does our code base use the optional second parameter anywhere?  If not, maybe 
it would be even simpler to eliminate the optional second parameter, and just 
call

      warning (_f (. . .), origin);

and keep all the testing for a null pointer in the warning function.

However, I see that this could break existing uses, so it might be better to 
use your helper function.  But maybe the name source_location would be more 
clear than message_location (since we aren't displaying the location of a 
message, but the location in the source file of the bit that caused the 
message, IIUC).

Thanks,

Carl