haowei added a comment. @brettw Thanks for the clarification. I was oncall for buildgardener last week and got quite busy due to breakages.
Thanks for your clarification, that is really helpful to understand the your intent. While I still don't see the way these constructors are implemented is an issue. I don't have a strong opinions on this and OK with your refactors. "semantically equivalent" means these test code should have same input/output, performance same routines of task and use same amount of memory. Before your change, these typeinfo's symbolID in the test are always pointing to a constant EmptySID, after your change, some of these typeinfo points to an empty symbol ID in allocated in memory when typeinfo is created. While the output is the same, they are not equivalent. For refactor changes, I would prefer to avoid introducing such changes. I have one question about deleting the IsInGlobalNamespace. I understand it is not well implemented and has a bug in its constructor, but I don't think we should simply delete it just because it is buggy and not well used. It is part of C++ and commonly used. Do you have any plan to add it back after all the refactor work finishes? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134235/new/ https://reviews.llvm.org/D134235 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
