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

C++: hawkmoth warns about #pragma once in main file #208

Closed
stephanlachnit opened this issue Nov 7, 2023 · 15 comments · Fixed by #213
Closed

C++: hawkmoth warns about #pragma once in main file #208

stephanlachnit opened this issue Nov 7, 2023 · 15 comments · Fixed by #213
Labels
parser Related to the Clang glue layer

Comments

@stephanlachnit
Copy link
Contributor

hawkmoth (or more likely, clang) warns about #pragma once in main file.

To reproduce, try to add documentation for this file:

#pragma once

/*
 * Test doc
 */
void test_function() {}

The warning has no impact on the output.

@stephanlachnit stephanlachnit changed the title C++:hhawkmoth warns about #pragma once in main file C++: hawkmoth warns about #pragma once in main file Nov 7, 2023
@jnikula
Copy link
Owner

jnikula commented Nov 8, 2023

It does work for me, though.

What's your platform and clang --version?

@jnikula jnikula added the parser Related to the Clang glue layer label Nov 8, 2023
@jnikula
Copy link
Owner

jnikula commented Nov 8, 2023

Also, what's the warning, exactly? If it can be reproduced on the hawkmoth command-line, all the better.

@stephanlachnit
Copy link
Contributor Author

What's your platform and clang --version?

Debian clang version 16.0.6 (17)
Target: x86_64-pc-linux-gnu

On Debian Sid.

Also, what's the warning, exactly? If it can be reproduced on the hawkmoth command-line, all the better.

Interestingly, this does not appear when I run hawkmoth --verbose --domain cpp test.hpp, only when I add this file to sphinx via cpp:autodoc.

@jnikula
Copy link
Owner

jnikula commented Nov 8, 2023

Interestingly, this does not appear when I run hawkmoth --verbose --domain cpp test.hpp, only when I add this file to sphinx via cpp:autodoc.

Oh? I only tested on the command-line, I'll need to try via the Sphinx extension.

@jnikula
Copy link
Owner

jnikula commented Nov 8, 2023

Looks like this is related to having #pragma once in a file with suffix .c or .cpp i.e. not as a header guard in .h or .hpp.

Please either avoid doing that, or add -Wno-pragma-once-outside-header to hawkmoth_clang https://jnikula.github.io/hawkmoth/stable/extension.html#hawkmoth_clang

$ echo "#pragma once" > test.hpp
$ echo "#pragma once" > test.cpp
$ hawkmoth --domain=cpp test.hpp
$ hawkmoth --domain=cpp test.cpp
WARNING: test.cpp:1: #pragma once in main file
$ hawkmoth --domain=cpp --clang=-Wno-pragma-once-outside-header test.cpp

@stephanlachnit
Copy link
Contributor Author

Looks like this is related to having #pragma once in a file with suffix .c or .cpp i.e. not as a header guard in .h or .hpp.

Nope, it definitely does for me when I use it via sphinx:

/home/stephan/Projects/constellation_asio_sd/docs/source/api.rst:9: WARNING: /home/stephan/Projects/constellation_asio_sd/test.hpp:1: #pragma once in main file

But I can confirm that renaming test.hpp to test.cpp and running hawkmoth via CLI does trigger it.

@jnikula
Copy link
Owner

jnikula commented Nov 9, 2023

Okay, I can reproduce the behaviour, but I'm pretty confused why the result is different via CLI vs. Sphinx/pytest. It's reproducible also in the CLI test that gets run via pytest. I understand the warning, but not the difference.

There's basically three ways to work around this:

  • Passing -Wno-pragma-once-outside-header to Clang
  • Passing -xc-header or -xc++-header to Clang when parsing .h or .hpp
  • Using the traditional #ifndef header guards. (Unsolicited opinion, I'd always go for that instead of #pragma once, because the pragma is non-standard and can lead to issues.)

Hawkmoth could identify headers and pass the -x option, but I'd prefer not doing that until we've figured out where the difference comes from. Maybe there's a way to make the libclang detection work as it should instead.

@stephanlachnit
Copy link
Contributor Author

If it this a clang issue, then I'm perfectly fine with adding -Wno-pragma-once-outside-header to the compile arguments. Thanks for looking into it!

@stephanlachnit
Copy link
Contributor Author

stephanlachnit commented Nov 9, 2023

Edit: this seams unrelated

It seems to be a bit ore interesting than just pragma once. For example, this code:

#include <exception>
#include <string>

namespace cnstln {
namespace CHIRP {

/**
 * Error thrown when a Message was not decoded successfully
*/
class DecodeError : public std::exception {
public:
    /**
     * @param error_message Error message
    */
    DecodeError(std::string error_message) : error_message_(std::move(error_message)) {}
    /**
     * @return Error message
    */
    const char* what() const noexcept override { return error_message_.c_str(); }
protected:
    /** The error message */
    std::string error_message_;
};

}
}

Produces no errors with the CLI, but gives

Exception occurred:
  File "/home/stephan/Projects/constellation_asio_sd/venv/lib/python3.11/site-packages/clang/cindex.py", line 650, in from_id
    raise ValueError('Unknown template argument kind %d' % id)
ValueError: Unknown template argument kind 604

via sphinx (using #211, but the error is happens after cursor.kind == CursorKind.UNEXPOSED_DECL)

@stephanlachnit
Copy link
Contributor Author

Okay, I can reproduce the behaviour, but I'm pretty confused why the result is different via CLI vs. Sphinx/pytest. It's reproducible also in the CLI test that gets run via pytest. I understand the warning, but not the difference.

I found the bug, it comes from this lines:

clang_args = ['-xc++'] if self._domain == 'cpp' else []

I think this line can simply be removed, at least as far as my testing goes.

@jnikula
Copy link
Owner

jnikula commented Nov 9, 2023

Are you sure your libclang and its python bindings are of the same version? ValueError: Unknown template argument kind 604 does not seem like something Hawkmoth could trigger.

@jnikula
Copy link
Owner

jnikula commented Nov 9, 2023

Edit: this seams unrelated

Agreed.

@stephanlachnit
Copy link
Contributor Author

Agreed.

FYI it looks like a C++20 bug in clang 16.

stephanlachnit added a commit to stephanlachnit/hawkmoth that referenced this issue Nov 9, 2023
By default, hawkmoth added the -xc++ compiler argument to clang, which caused issues with pragma only declaration (See jnikula#208). Changig this to -xc++-header solves this issue, while retaining compatibility with compiled units.

Signed-off-by: Stephan Lachnit <[email protected]>
stephanlachnit added a commit to stephanlachnit/hawkmoth that referenced this issue Nov 9, 2023
By default, hawkmoth added the -xc++ compiler argument to clang, which caused issues with pragma only declaration (See jnikula#208). Changig this to -xc++-header solves this issue, while retaining compatibility with compiled units.

Signed-off-by: Stephan Lachnit <[email protected]>
jnikula added a commit that referenced this issue Nov 9, 2023
Have a single point of truth for setting the clang -x<language> option,
to unify parsing across the extension, tests, and cli.

Choose the language primarily based on the domain, and secondarily based
on the filename extension to differentiate headers. Using "-header" is
necessary for some cases such as "#pragma once", where plain "-xc" or
"-xc++" would interpret the file as a main file.

This unconditionally bypasses libclang automatic detection. The user and
tests can still override the parser selected language, as the user
provided clang_args are appended.

Fixes: #208
@jnikula
Copy link
Owner

jnikula commented Nov 9, 2023

Okay, our -x<language> option usage was pretty inconsistent. I've hopefully fixed it in #213.

jnikula added a commit that referenced this issue Nov 9, 2023
Have a single point of truth for setting the clang -x<language> option,
to unify parsing across the extension, tests, and cli.

Choose the language primarily based on the domain, and secondarily based
on the filename extension to differentiate headers. Using "-header" is
necessary for some cases such as "#pragma once", where plain "-xc" or
"-xc++" would interpret the file as a main file.

This unconditionally bypasses libclang automatic detection. The user and
tests can still override the parser selected language, as the user
provided clang_args are appended.

Fixes: #208
@jnikula
Copy link
Owner

jnikula commented Nov 9, 2023

@stephanlachnit Many thanks for the report and debugging! There was more to this than met the eye at first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser Related to the Clang glue layer
Projects
None yet
2 participants