[clang] [clang][NFCI] Clarify ownership of PragmaHandlers (PR #117703)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > Can you share what the Static assert complaint is that makes you think we should be doing this?
> 
> The complaint is that `Preprocessor::addPragmaHandler` called from parser with a raw ptr argument coming from a `unique_ptr` owned by Parser actually initializes another `unique_ptr` with that same raw ptr (the second `unique_ptr` is stored in a `PragmaNamespace` which is owned by `Preprocessor`). That results in two `unique_ptr`s that manage the same memory at the same time. Which seems dangerous. This also creates a requirement to not forget to call `removePragmaHandler` at the right time, otherwise the two `unique_ptr`s may cause double free.

This is definitely concerning.  But because we're properly managing lifetime, it seems sensible to me to just have `PragmaNamespace` actually just store a pointer or reference to the `unique_ptr` instead.  

> It seems I can't get rid of PragmaHandlers in Parser, since some of the handlers are "added" to Preprocessor under different namespaces using the same pointer owned by Parser (example UnrollHintHandler). It seems I can't remove a smart pointer from `PragmaNamespace` either since Preprocessor itself adds some handlers via `addPragmaHandler` (see `Preprocessor::RegisterBuiltinPragmas`) which are not stored anywhere. The two `unique_ptr`s created for the same raw pointer makes me think it should be a `shared_ptr` instead.

The problem with `shared_ptr` is it creates a separation of ownership that we don't really mean.  As you mentioned (and I can see now), we're conflating ownership in a poor way.  However, `shared_ptr` instead makes it so that the consequence of forgetting to unregister (which, IMO, is part of our ownership model) is now `fine` because it gets destroyed with whoever remembers to undo it last.  At least before we were warned about it by a double-free during runtime.

I would vastly prefer the destructor of the thing that needs to have `removePragmaHandler` assert if it hasn't been unregistered, then leave it as a `unique_ptr` in the Parser.

https://github.com/llvm/llvm-project/pull/117703