Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate definition with C "typedef struct foo foo" #543

Open
Shamino0 opened this issue Jun 11, 2020 · 3 comments
Open

Duplicate definition with C "typedef struct foo foo" #543

Shamino0 opened this issue Jun 11, 2020 · 3 comments
Labels
bug Problem in existing code

Comments

@Shamino0
Copy link

Shamino0 commented Jun 11, 2020

The following concept is perfectly valid C code:

typedef struct foo_t {
   int bar;
} foo_t;

For a C++ project, of course, the typedef is redundant, but for a C project, it is not. foo_t and struct foo_t are two distinct items.

Doxygen renders this correctly, putting the foo_t documentation in a "Typedef Documentation" section and struct foo_t in a "Data Structures" section. The typedef has a link to the structure.

When I use Breathe to embed the header's documentation into a Sphinx file with

.. doxygenfile:: foo.h
   :project: MYPROJECT

it generates a warning, saying that foo_t is a duplicate definition:

.../mydoc.rst: 9: WARNING: Duplicate declaration, foo_t

In the Sphinx-generated HTML output, I do see the two items listed in their proper sections. foo_t in the "Typedefs" section and struct foo_t in the (unlabeled) structures section, but I can only link to the typedef. References to :c:type:`foo_t` correctly links to the foo_t typedef, but references to :c:struct:`foo_t` links to the same location (it should link to the struct foo_t structure).

This problem does not happen if the struct and the typedef use different symbols:

typedef struct foo_s {
   int bar;
} foo_t;

but I shouldn't have to make such a change in my code. It is perfectly valid C for the struct name and the typedef name to be the same.

I also understand that this should be a problem in C++, where the typedef is redundant. But I've explicitly configured Breathe to use the C domain, so it should know that this is not C++ code:

breathe_domain_by_extension = { "h" : "c" }

Somewhere in Breathe, the code does not distinguish between different C language constructs that share the same name. For example, all of the following are distinct language elements:

  • typedef ... foo_t - referenceable via :c:type:`foo_t`
  • struct foo_t - referenceable via :c:struct:`foo_t`
  • union foo_t - referenceable via :c:union:`foo_t`
  • enum foo_t - referenceable via :c:enum:`foo_t`

I'm not sure what the best fix is, but please see if there is something that can be done.

@jakobandersen
Copy link
Collaborator

There may be a minor change needed in Breathe, but otherwise the core problem is in Sphinx. I have opened an issue over there to discuss what the behaviour should be.
I tried double checking that this is not a regression with Sphinx v3.x and and the newer versions of Breathe, but in this example I couldn't get it to insert the struct, only the typedef (though in a raw Sphinx file it seems indeed like it is not a regression).
Can you try in your project with Sphinx 2.4.4 and an older Breathe version (v4.14.2?) and see which results you get? It may not spit out a duplication warning, but when you hover near a duplicate declaration in the HTML there will be no permalink shown.

@Shamino0
Copy link
Author

Thanks. You are correct - it appears to be a Sphinx problem. I produced a test-case (attached) that demonstrates this (tested with Sphinx 3.1.0 and Breathe 4.19.0)

testcase.zip

I'll see if I can revert to older versions and see if anything changes.

@Shamino0
Copy link
Author

Unfortunately, installing older versions doesn't seem to work. Installing Sphinx 2.4.4 via pip3 seems to end up installing 3.1.0 anyway (or something broken that reports itself as 3.1.0). Likewise with older versions of Breathe. I'm not sure what the cause is, but the result crashes:

Running Sphinx v3.1.0
making output directory... done
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 3 source files that are out of date
updating environment: [new config] 3 added, 0 changed, 0 removed
reading sources... [ 33%] foo                                                                                                                                
Exception occurred:
  File "/home/qca/.local/lib/python3.6/site-packages/sphinx/domains/c.py", line 3093, in object_type
    raise NotImplementedError()
NotImplementedError
The full traceback has been saved in /tmp/sphinx-err-bpeadj_k.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
Makefile:25: recipe for target 'html' failed
make: *** [html] Error 2

shaunrd0 added a commit to TileDB-Inc/TileDB that referenced this issue Jul 12, 2022
+ Add c_id_attributes to conf.py for parsing TILEDB_*
+ Rename typedef tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ breathe-doc/breathe#543
shaunrd0 added a commit to TileDB-Inc/TileDB that referenced this issue Jul 12, 2022
+ Add c_id_attributes to conf.py for parsing TILEDB_*
+ Rename typedef tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ breathe-doc/breathe#543
ihnorton pushed a commit to TileDB-Inc/TileDB that referenced this issue Aug 25, 2022
* Add C API doc for tiledb_array_upgrade_version

* Configuration changes

+ Add c_id_attributes to conf.py for parsing TILEDB_*
+ Rename typedef tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ breathe-doc/breathe#543

* Refactor changes

+ Rename struct tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ typedef tiledb_experimental_query_status_details_t tiledb_query_status_details_t
shaunrd0 added a commit to TileDB-Inc/TileDB that referenced this issue Sep 20, 2022
* Add C API doc for tiledb_array_upgrade_version

* Configuration changes

+ Add c_id_attributes to conf.py for parsing TILEDB_*
+ Rename typedef tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ breathe-doc/breathe#543

* Refactor changes

+ Rename struct tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ typedef tiledb_experimental_query_status_details_t tiledb_query_status_details_t
shaunrd0 added a commit to TileDB-Inc/TileDB that referenced this issue Sep 20, 2022
* Add C API doc for tiledb_array_upgrade_version

* Configuration changes

+ Add c_id_attributes to conf.py for parsing TILEDB_*
+ Rename typedef tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ `breathe-doc/breathe#543

* Refactor changes

+ Rename struct tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ typedef tiledb_experimental_query_status_details_t tiledb_query_status_details_t
ihnorton pushed a commit to TileDB-Inc/TileDB that referenced this issue Sep 20, 2022
* Add C API doc for tiledb_array_upgrade_version

* Configuration changes

+ Add c_id_attributes to conf.py for parsing TILEDB_*
+ Rename typedef tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ `breathe-doc/breathe#543

* Refactor changes

+ Rename struct tiledb_query_status_details_t to tiledb_experimental_query_status_details_t
+ typedef tiledb_experimental_query_status_details_t tiledb_query_status_details_t
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Problem in existing code
Projects
None yet
Development

No branches or pull requests

3 participants