Skip to content

[C++20] [Modules] Confusing error message about invisible namespace #73893

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

Closed
ChuanqiXu9 opened this issue Nov 30, 2023 · 4 comments
Closed

[C++20] [Modules] Confusing error message about invisible namespace #73893

ChuanqiXu9 opened this issue Nov 30, 2023 · 4 comments
Assignees
Labels
clang:modules C++20 modules and Clang Header Modules

Comments

@ChuanqiXu9
Copy link
Member

I got this issue report in a private chat and I think it is better to track it.

The reproducer:

// tm.cppm
module;
#include <vector>
export module tm;

export void mf();

// tc.cpp
import tm;

int main()
{
    std::printf("main\n");
    mf();
}

then when we compile tc.cpp, we'll see the error message like:

tc.cpp:16:5: error: missing '#include <vector>'; 'std' must be declared before it is used
   16 |     std::printf("main\n");
      |     ^
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/vector:104:11: note: declaration here is not visible
  104 | namespace std _GLIBCXX_VISIBILITY(default)
      |           ^
tc.cpp:16:10: error: no member named 'printf' in namespace 'std'
   16 |     std::printf("main\n");
      |     ~~~~~^
2 errors generated.

The issue reporter's opinion is that, the header vector in the error message is confusing since there is nothing about <vector>.

The reason for the issue is that, in the diagnostic handler, we don't handle namespace decl specially. So that we will show the first declaration and say it is invisible to imply the users to export that. But it is confusing for namespace decl since it smells not good for users to export a whole namespace.

Also, once a declaration in the namespace got exported, the namespace become visible too. So the solution in my mind may be, we can simply don't diagnose anything if the declaration is a namespace declaration. Then the showed error message will be:

tc.cpp:16:10: error: no member named 'printf' in namespace 'std'
   16 |     std::printf("main\n");
      |     ~~~~~^
@ChuanqiXu9 ChuanqiXu9 added the clang:modules C++20 modules and Clang Header Modules label Nov 30, 2023
@ChuanqiXu9 ChuanqiXu9 self-assigned this Nov 30, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 30, 2023

@llvm/issue-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)

I got this issue report in a private chat and I think it is better to track it.

The reproducer:

// tm.cppm
module;
#include &lt;vector&gt;
export module tm;

export void mf();

// tc.cpp
import tm;

int main()
{
    std::printf("main\n");
    mf();
}

then when we compile tc.cpp, we'll see the error message like:

tc.cpp:16:5: error: missing '#include &lt;vector&gt;'; 'std' must be declared before it is used
   16 |     std::printf("main\n");
      |     ^
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/vector:104:11: note: declaration here is not visible
  104 | namespace std _GLIBCXX_VISIBILITY(default)
      |           ^
tc.cpp:16:10: error: no member named 'printf' in namespace 'std'
   16 |     std::printf("main\n");
      |     ~~~~~^
2 errors generated.

The issue reporter's opinion is that, the header vector in the error message is confusing since there is nothing about &lt;vector&gt;.

The reason for the issue is that, in the diagnostic handler, we don't handle namespace decl specially. So that we will show the first declaration and say it is invisible to imply the users to export that. But it is confusing for namespace decl since it smells not good for users to export a whole namespace.

Also, once a declaration in the namespace got exported, the namespace become visible too. So the solution in my mind may be, we can simply don't diagnose anything if the declaration is a namespace declaration. Then the showed error message will be:

tc.cpp:16:10: error: no member named 'printf' in namespace 'std'
   16 |     std::printf("main\n");
      |     ~~~~~^

@davidstone
Copy link
Contributor

I've been meaning to report this (or maybe something like this?) as well, but I hadn't come up with a good solution. I'm regularly told to import the wrong module -- "Oh, you tried to use foo::bar? I know about a module that contains namespace foo, you should import that!"

If you need a reproducer for that (since I'm not sure if it's exactly the same as this), I can make one the next time I encounter it.

@ChuanqiXu9
Copy link
Member Author

I've been meaning to report this (or maybe something like this?) as well, but I hadn't come up with a good solution. I'm regularly told to import the wrong module -- "Oh, you tried to use foo::bar? I know about a module that contains namespace foo, you should import that!"

How do you think the proposed solution? "To not diagnose on the namespace". Then for the your example, the message becomes to "bar is invisible" or "unknown identifier bar" or similar messages. Personally I feel such message are helpful enough.

If you need a reproducer for that (since I'm not sure if it's exactly the same as this), I can make one the next time I encounter it.

I guess they'll be the same issue. But more reproducer can increase our confidence.

@davidstone
Copy link
Contributor

Your solution sounds great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules
Projects
None yet
Development

No branches or pull requests

3 participants