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++20] [Modules] ADL does not work properly for dependent expressions #60488

Closed
davidstone opened this issue Feb 2, 2023 · 4 comments
Closed
Assignees
Labels
clang:modules C++20 modules and Clang Header Modules

Comments

@davidstone
Copy link
Contributor

davidstone commented Feb 2, 2023

Given the following two valid files:

module;

namespace n {

struct s { };

void operator+(s, int) {
}

} // namespace n

export module a;

export template<typename T>
void a(T x) {
	n::s() + x;
}
module;

export module b;

import a;

void b() {
	a(0);
}

When compiled with

clang++ -std=c++20 -x c++-module a.cpp --precompile -o a.pcm
clang++ -std=c++20 -x c++-module b.cpp --precompile -o b.pcm -fmodule-file=a.pcm

causes Clang to fail with

In file included from b.cpp:1:
a.cpp:16:9: error: invalid operands to binary expression ('n::s' and 'int')
        n::s() + x;
        ~~~~~~ ^ ~
b.cpp:8:2: note: in instantiation of function template specialization 'a<int>' requested here
        a(0);
        ^
1 error generated.

In practice, this comes up with code like std::string() + some_custom_to_string(my_type). Calling code needs to #include <string> in their module, even though they know nothing about std::string in the called function.

@EugeneZelenko EugeneZelenko added clang:modules C++20 modules and Clang Header Modules and removed new issue labels Feb 2, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 2, 2023

@llvm/issue-subscribers-clang-modules

@ChuanqiXu9
Copy link
Member

This looks like a bug indeed. BTW, as a workaround, you can export the operator +() in practice. For example:

module;

namespace n {

struct s { };

void operator+(s, int) {
}

} // namespace n

export module a;

export template<typename T>
void a(T x) {
	n::s() + x;
}

namespace n {
     export using n::operator+;
}

@ChuanqiXu9 ChuanqiXu9 self-assigned this Feb 3, 2023
@davidstone
Copy link
Contributor Author

Here's a second test case that might be the same problem, might be a different problem. In this example, the dependent expression occurs slightly differently. It looks like this needs the global module fragment to trigger:

export module a;

export template<typename T>
void a(T x) {
	+x;
}
module;

struct s {
};
void operator+(s) {
}

export module b;

import a;

export template<typename T>
void b() {
	a(s());
}
export module c;

import b;

void c() {
	b<int>();
}

when compiled with

clang++ -std=c++20 -x c++-module a.cpp --precompile -o a.pcm
clang++ -std=c++20 -x c++-module b.cpp --precompile -o b.pcm -fmodule-file=a.pcm
clang++ -std=c++20 -x c++-module c.cpp --precompile -o c.pcm -fmodule-file=b.pcm

fails with

In module 'b':
a.cpp:5:2: error: invalid argument type 's' to unary expression
        +x;
        ^~
b.cpp:14:2: note: in instantiation of function template specialization 'a<s>' requested here
        a(s());
        ^
c.cpp:6:2: note: in instantiation of function template specialization 'b<int>' requested here
        b<int>();
        ^
1 error generated.

This is reduced from comparing std::string::iterator.

@ChuanqiXu9
Copy link
Member

Update: the corresponding of the wording is http://eel.is/c++draft/module.context#3. We didn't implement modules' Instantiation context before. So here is the problem.

CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Feb 8, 2023
Close llvm/llvm-project#60488.

Previously, when we instantiate a template, the argument dependent
lookup is performed in the context of the instantiation, which implies that the
functions not visible in the context can't be found by the argument
dependent lookup.

But this is not true, according to [module.context]p3, the instantiation
context for the implicit instantiation of a template should contain the
context of the primary module interface if the template is defined in
the module interface unit.

Note that the fix didn't implemnet [module.context]p3 precisely, see the
comments for example.
skatrak pushed a commit to skatrak/llvm-project-rocm that referenced this issue Feb 10, 2023
Close llvm/llvm-project#60488.

Previously, when we instantiate a template, the argument dependent
lookup is performed in the context of the instantiation, which implies that the
functions not visible in the context can't be found by the argument
dependent lookup.

But this is not true, according to [module.context]p3, the instantiation
context for the implicit instantiation of a template should contain the
context of the primary module interface if the template is defined in
the module interface unit.

Note that the fix didn't implemnet [module.context]p3 precisely, see the
comments for example.
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

4 participants