On Mon, Sep 26, 2022 at 1:42 PM Haowei Wu via Phabricator < revi...@reviews.llvm.org> wrote:
> 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? > I guess I'll ask you: what does this boolean mean and why do we need it? This flag was only ever derived from the "path" which is just a member in the same class. Users already have the full namespace information as far as I can tell. Brett
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
