[lldb-dev] [Bug 15301] New: LLDB prints incorrect size of libstdc++ vector<bool> containers (when inferior built with GCC on Linux)

[Yacine Belkadi]

Hi Enrico,

On 05/15/2013 11:32 PM, Enrico Granata wrote:
> Yacine,
> 
> In addition to what Greg said, I am also slightly confused by your
> FormatManager changes.
> Two points here:
> 
> 1) a regular expression is not really necessary. If I understand the scope
> of the GCC bug correctly, all we need to do is match the two *exact* strings
> std::vector<std::allocator<bool>> and std::vector<bool,
> std::allocator<bool>, bool, std::allocator<bool> >
> to catch all cases
> Regexp matching is slower and it is easy to write a regexp that over-reaches
> and matches more types than we would like to
> If this is a workaround for a bug rather than a necessity, I would rather
> much keep it as confined as possible, so I would definitely rework in terms
> of exact matching

Adding an exact string to match "std::vector<bool, std::allocator<bool>,
bool, std::allocator<bool> >" would indeed serve as a workaround, but only
for the specific case of vector<bool> + the GCC version with the bug +
"frame variable". It will not be enough to match the "std::vector<bool,
std:allocator<bool> >" that GCC initially intended to produce for
"std::vector<bool>". It will not avoid problems with the erroneous parsing
of other types that have templates parameters that will get duplicated. So I
think the first patch was important to have a complete workaround.

As I needed to match two similar types:
"std::vector<bool, std:allocator<bool> >" (produced by GCC) and
"std::vector<std:allocator<bool> >" (produced by Clang), I thought it was
more appropriate to factor the two into the regex
"^std::vector<(bool,( )?)?std::allocator<bool> >$".

> 2) It looks like you are removing the summary for a vector of bools
> entirely. Why?
> You would just need to add the same summary with a new typename to the
> category, instead your patch at lines 42 and 43 is removing the creation of
> the summary for std::vector<bool> and I can’t see the replacement

I removed it because it looked redundant, as it's already included in the
regex summary for other vectors, on line 572.

> This is mostly a minor cosmetic issue, and it’s not even enforced
> consistently in the code, but I tend to add new formatters using
> Add/Formatter /calls instead of directly playing with the FormatNavigator
> objects
> It’s kind of laying groundwork for potentially reworking the way built-in
> formatters are added to an easier to maintain coding style. I have a couple
> ideas there, just not enough time to code them :-)

Oh yes, I should have used that. It's a lot cleaner.

Thanks for you review.
Yacine

> Thanks for clarifying these data points.
> 
> Enrico Granata
> 📩 egranata@.com
> ☎️ 27683