Re: [PATCH v2 4/4] Add doc and news for DWARF index cache

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2018-07-27 04:50, Eli Zaretskii wrote:
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> CC: Simon Marchi <simon.marchi@ericsson.com>
>> Date: Wed, 25 Jul 2018 18:47:04 -0400
>> 
>> This patch adds doc and news for the feature introduced by the 
>> previous
>> patch.
> 
> Thanks.
> 
>> +* DWARF index cache: GDB can now automatically save indices DWARF 
>> symbols on
> 
> "indices of DWARF symbols", I presume?

Yes, thanks.

>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index b36a39b..9533c72 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -20095,6 +20095,42 @@ There are currently some limitation on 
>> indices.  They only work when
>>  for DWARF debugging information, not stabs.  And, they do not
>>  currently work for programs using Ada.
>> 
>> +@subsection Automatic symbol index cache
>> +
>> +It is possible for GDB to automatically save a copy of this index in 
>> a cache
>                       ^^^
> @value{GDBN}

Fixed.

>> +on disk and retrieve it from there when loading the same binary in 
>> the future.
>> +This feature can be turned on with @command{set index-cache on}.  The 
>> following
> 
> @command is incorrect here, it's the markup for shell commands, like
> 'ls'.  What you want is @kbd.

Ok, fixed.

>> +@table @code
>> +
>> +@item set index-cache on
>> +@itemx set index-cache off
>> +
>> +Enable or disable the use of the symbol index cache.
> 
> There should be no empty line between @item and the following
> description.

Ok.

>> +
>> +@item set index-cache directory @var{directory}
>> +@itemx show index-cache directory
>> +Set/show the directory where index files will be saved.  By default, 
>> the value
>> +@code{$XDG_CACHE_HOME/gdb} is used if the @code{XDG_CACHE_HOME} 
>> environment
>> +variable is defined.  The value @code{$HOME/.cache/gdb} is used 
>> otherwise.
> 
> Please don't use $FOO to mean an environment variable, that is a Unix
> shell convention.  I suggest to rephrase:
> 
>   By default, the index is cached in the @file{gdb} subdirectory of
>   the directory pointed to by the @env{XDG_CACHE_HOME} environment
>   variable, if it is defined, else in the @file{.cache/gdb}
>   subdirectory of your home directory.

Ok, done.

>> +@item set index-cache format @var{format}
>> +@itemx show index-cache format
>> +Set/show the format in which index files are saved.  @var{format} can 
>> be either
>> +@code{gdb} (the default) or @code{dwarf-5}.  Note that @value{GDBN} 
>> is currently
>> +only able to read back files in the @code{gdb} format from the cache, 
>> so
>> +@code{dwarf-5} is not very useful.
> 
> If 'dwarf-5' cannot be used, why are we documenting it, and why are we
> documenting/implementing this command in the first place?

Good point.  It's on my road map to add support for reading back DWARF-5 
files from the index, but since it's a bit more complex I'm keeping it 
for another patch.

It's true that it doesn't really make sense to include that setting now, 
so I'll probably just remove it in the next version of the series.

>> +@item show index-cache stats
>> +Print the number of cache hits and misses for the index cache since 
>> the launch
>> +of @value{GDBN}.
> 
> This begs the question: for which index cache will this show the
> statistics?  For the one defined by the latest "set index-cache
> directory" command?

Ah, no it's dumber than that.  It's just how many hits and misses there 
were since GDB was launched, regardless of the cache directory used.  
Perhaps it would be clearer to keep it short like:

   Print the number of cache hits and misses since the launch of 
@value{GDBN}.

?

Thanks,

Simon