@file and @headername directives

[Jonathan Wakely]

On Sun, 18 Aug 2024, 14:31 Kim Gräsman, <kim.grasman@gmail.com> wrote:

> Thanks, that helps!
>
> On Sun, Aug 18, 2024 at 2:17 PM Jonathan Wakely <jwakely.gcc@gmail.com>
> wrote:
>
>> Doxygen figures out the filename in most cases, so @filename isn't needed.
>>
>
> I didn't know these were Doxygen directives, that actually helps clarify a
> lot :-)
>
> > * @file sometimes has a filename distinct from the file it's actually in
>>
>> Which cases below? Do you mean the "include/algorithm" ones?
>> That is part of the actual filename though, so it's not distinct.
>> Doxygen is run on the build tree, not the source tree, and so the
>> headers such as include/std/foo are symlinked to include/foo, so the
>> @filename is correct.
>>
>
> The one I ran into was:
> /usr/include/x86_64-linux-gnu/c++/11/ext/opt_random.h
>
> in my install tree, it identifies as "@file ext/random.tcc", which is
> probably also just a tiny bug.
>

Ah yes, that looks like a copy & paste bug from another file. We should fix
that one too, thanks!

>
> > * @file often has a spurious "include/" before the filename
>>
>> IIRC it's needed in some cases, to disambiguate include/foo from
>> include/ext/foo or include/tr2/foo
>>
>
> Aha. I thought they were literally the include-name the user would use
> from the install tree, i.e. "foo" vs. "ext/foo" vs "tr2/foo", but I can see
> that Doxygen gets confused by an unqualified name.
>

It certainly used to get confused ... maybe that's been fixed in a more
recent doxygen but we'd have to check.

You'd think that the @file block would default to documenting *the file
it's in* but I think there was some problem where it doesn't when the name
of the file it's in is non-unique. Don't quote me on that, I might have
misremembered the details, but we have definitely had to fight against
doxygen over the years!


> Given how most headers don't have the "include/" prefix, I thought it was
> just copy/pasta.
>

Some of them probably are :-) like the std_abs.h one, which would be my
fault when I added that file.


> > --
>> > # include-what-you-use over a .cc file including all headers without
>> extension from /usr/include/c++/11
>> > /usr/include/c++/11/algorithm:51:10: warning: @file directive does not
>> match actual filename: include/algorithm vs. algorithm
>>
>> This check seems wrong (it does match the last two components of the
>> filename) and is needed to disambiguate <algorithm> from
>> <experimental/algorithm>.
>>
>
> Yeah, with the Doxygen semantics in the build tree. In the install tree,
> assuming "qualified" names, /algorithm is obviously different from
> /experimental/algorithm.
>

Yeah. Once upon a time I thought the "include/" was obviously wrong or
redundant and tried to remove it ... but doxygen wasn't happy.



>
>> > /usr/include/c++/11/bits/cpp_type_traits.h:27:52: warning: @headername
>> file does not exist: ext/type_traits (/usr/include/c++/11/ext/type_traits)
>>
>> That seems like a bug.
>>
>
> OK, I can cook up a patch for this if you want?
>

That would be great, thanks.


>
>> > /usr/include/c++/11/bits/predefined_ops.h:25:10: warning: @file
>> directive does not match actual filename: predefined_ops.h vs.
>> bits/predefined_ops.h
>>
>> "predefined_ops.h" is a unique filename, so this is sufficient for
>> Doxygen to identify it. Adding the bits/ prefix should work too, but
>> somebody would need to check.
>>
>
> Right. I think it will be hard to replicate Doxygen semantics in IWYU --
> we don't have the global context of all available libstdc++ headers when
> looking at a single file. I'd be happy to provide patches to make all @file
> directives match the expected include-name (i.e. predefined_ops.h is always
> included as "<bits/predefined_ops.h>"), but I can see that that's probably
> just busy-work from libstdc++'s point of view.
>

I wouldn't be opposed to it though. IWYU is a nice tool and I'd love if it
worked well with our headers. If being a bit more regular would help you,
even if it's not strictly needed for doxygen, that seems like a reasonable
change. As long as doxygen still works of course.

Ideally we'd be able to add tests to check that we don't regress in future
by introducing new headers that don't follow the new convention. But
failing that it would be OK to just document the convention.


>
>> I haven't got time to go through the rest of the warnings now, but
>> apart from the ext/type_traits.h bug most of them seem like our
>> Doxygen comments are correct (i.e. working as intended) and your tool
>> is too picky.
>>
>
> Most definitely. Since I wasn't aware these were Doxygen comments, I made
> some assumptions about semantics.
>
> Thanks,
> - Kim
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20240818/0aa5bcec/attachment-0001.htm>