[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

Sun Jul 17 03:20:58 PDT 2022

mizvekov added a comment.

In D112374#3658059 <https://reviews.llvm.org/D112374#3658059>, @kimgr wrote:

> Haha. Pun intended? :-)

Yes :-)

> As you noticed, it's not our tests that care about the AST, it's the tool itself. IWYU has been around since 2010-11, so there's probably lots of code in there to work around bugs and idiosyncrasies in the Clang AST that have since been fixed. I've inherited the project, so I don't have much information on how or why the implementation ended up the way it did.
>
> Anyway, thanks for the heads-up about `getAs` vs `dyn_cast`, that seems like an easy transformation for us to do. Though I'm not sure where -- everywhere a `Type*` is processed?
>
> Also, I suspect we have a few open-ended searches where we're looking for the first desugared type in the tree, but I guess that's where `getCanonicalType` would be used?

I think for IWYU getCanonicalType could be problematic. It would be fine for quickly testing what kind of node you have under all that sugar and such, but if for example you try to get a Decl represented by some canonical type, you would likely get a canonical decl, but it seems to me that would not be useful because you might need to know the exact (re)-declaration used, which has source location information and you could pin down to a specific file.

There is `getDesugaredType`, which will just pull off all top level sugar without really canonicalizing the whole thing.

If instead you want to search down a type for the first thing of interest, then you can in that case have a main switch case on the type class, or even perhaps keep an if else chain of dyn_casts, but on your default or else case you could just test that the current type node is sugar with `getSingleStepDesugaredType` on it, see if you get a different result and try again with it in that case.
That way this mechanism does not get poisoned by clang introducing some new type sugar, which could not even be relevant to IWYU.

> Thanks!

Thank you for your patience as well, and sorry for the trouble!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374