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

[Haowei Wu via cfe-commits]

Please avoid directly replying to the email as they won't be logged in the
phabricator.

This boolean is logged into YAML output, while I don't have data, I could
bet there will be users depending on it even if it looks so unlikely. We
don't own the tool in the first place, in that case, I felt if there is a
bug in its constructor, we should fix the bug instead of abruptly deleting
it, even if it looks so easy to be derived from other values.

On Mon, Sep 26, 2022 at 3:14 PM Brett Wilson <brettw at gmail.com> wrote:

> 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/3603af3e/attachment-0001.html>