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

[Greg Clayton via lldb-commits]

clayborg added a comment.

> I thought you said the issue was if you try to merge to a branch that didn't have the `ClangUtil.cpp` file.  If you just copy and paste a function from one location in a file to another location and change the class it belongs to, that's still a problem?


So merging to current top of tree is easier than merging to a release branch. When merging to a top of tree for any repository that is currently mirroring a close relative of the current code is easy and the extra file doesn't matter. The issues arise when we find a bug deep in one of our released branches. Then to fix it we actually fix it into llvm.org SVN, then merge to swift.org into many branches, and then merge into many internal branches and eventually over to the release branch. We want minimal code changes so that we minimize our risk when taking fixes into release branches.

Anytime we introduce manual merging where we have to say "Oh, this function in 'ClangUtil::DoSomething' used to be in 'ClangASTContext::DoSomething', so I will need to take the guts of the function and manually copy it over and make sure it actually works". This is when errors happen. The other worse case is where we actually just merge the ClangUtil.cpp and ClangUtil.h file into our older branch, make sure it gets built in the Xcode project file, and then none of our code gets fixed because we still have the "ClangASTContext::DoSomething" that is buggy, but our merging worked just fine by merging in a new source that compiles but no one uses.

> That's a mighty big burden for upstream developers to have to deal with.


But it is where we are and it is our reality. So I am trying to minimize and point out when these kinds of issues will cause problems as I review patches. I know it is tempting to move stuff around, and we used to just accept these patches, but as we have over the past year, we have run into many issues of exactly the flavor we are talking about here, so I am proactively trying to avoid it when the need to do so isn't as strong. Many times I agree that things were architected incorrectly, or just are architected the way they are because of the way the code has evolved over the years, and those I believe we should fix. I would rather avoid this when we can when there isn't a strong reason to do so. I believe this is one of those cases.


http://reviews.llvm.org/D18530