[Lldb-commits] [PATCH] D18530: Move some functions from ClangASTContext to ClangUtil

[Zachary Turner via lldb-commits]

zturner added a comment.

One or two of the functions, you are right.  They do `clang::ASTContext` related things.  That was actually an oversight on my part.  I meant to move only functions that specifically did NOT do `clang::ASTContext` things.  Like `RemoveFastQualifiers`, or converting enums, or converting from qual types to canonical qual types.  Very simple utility functions.

The reason why I don't think they should be in `ClangASTContext` from a design point of view is because they are used from all over the places, including from places that aren't actually holding and don't need an `ASTContext`.

What if I move the `clang::ASTContext` functions back but keep the truly independent ones here?  `ClangASTContext` is a really egregiously oversized file, so I think splitting it up a bit would be helpful.  I also find it helpful to break a lot of these cyclic dependencies, where A.cpp includes B.h and B.cpp includes A.h, and we're doing that all over the place in large part because B.cpp is `ClangASTContext` and A.cpp is anything else that has to do anything, no matter what it is, to a clang type, even if it doesn't need an `ASTContext`.

What kind of merge conflict does it create?  There's no tricky changes here, why wouldn't they be mergeable to the other branch?  I'm not really crazy about the idea of having decisions about upstream being based on someone's downstream issues though :-/  If something is good for the upstream it seems like that's merit enough to get it in.


http://reviews.llvm.org/D18530