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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to