[PATCH] D73609: Change to individual pretty printer classes, remove generic `make_printer`.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/utils/gdb-scripts/prettyprinters.py:322-325
   try:
     enum_type = gdb.lookup_type(info_name + '::MaskAndShiftConstants')
   except gdb.error:
     return (None, None)
----------------
csigg wrote:
> dblaikie wrote:
> > csigg wrote:
> > > dblaikie wrote:
> > > > I think I asked this elsewhere, but not sure there was an answer (apologies if I forgot - might be worth a comment) - what is this error case for? Are there types in LLVM that don't provide this expected nested type?
> > > > 
> > > > Might be nice if it could be removed & then wouldn't need the factory layer of indirection for PointerIntPair and PointerUnion below.
> > > GDB gets confused about template arguments of 'std::_u::pair' (not sure I remember the exact name), and the lookup_type may fail.
> > > 
> > > Unfortunately GDB spews the console when a printer factory throws an exception, so it's better to catch it and handle it.
> > > 
> > > I will add a comment.
> > Where does std::pair come into this? The 4th template argument to PointerIntPair is the  PointerLikeTypeTraits, right?
> It's zero-based, so it's the `PointerIntPairInfo`, and if `PointerTy` is a `std::pair<...>*` gdb fails to look up the type. 
> 
> Something like that. Is it important enough to dig up the details?
Given the existence of this error path adds complexity to how this function is used, error paths to its callers, etc - yeah, I'd like to know why it's there & have a test case for it if it's needed (but hope there might be other solutions to addressing the underlying issue that might allow the error handling to be unnecessary/avoided), etc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73609/new/

https://reviews.llvm.org/D73609