[PATCH] D134235: [clang-doc] Clean up *Info constructors.

[Brett Wilson via cfe-commits]

On Mon, Sep 26, 2022 at 1:42 PM Haowei Wu via Phabricator <
reviews at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220926/7c377557/attachment.html>