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

Clang gives an error when using recursion in a concept #55875

Closed
s2s-k opened this issue Jun 4, 2022 · 8 comments · Fixed by #103867
Closed

Clang gives an error when using recursion in a concept #55875

s2s-k opened this issue Jun 4, 2022 · 8 comments · Fixed by #103867
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts

Comments

@s2s-k
Copy link

s2s-k commented Jun 4, 2022

MSVC: https://godbolt.org/z/M9YKEq6df
Clang: https://godbolt.org/z/rMM61PKKx

error: use of undeclared identifier 'IsAnyType'

I think the compiler gives this error for no reason, because "IsAnyType" has already been declared. Also, Microsoft's compiler compiles this code well.

@JoeLoser
Copy link
Member

JoeLoser commented Jun 4, 2022

I think this is an MSVC bug. Note that GCC rejects the code as well. This is because you can't have recursion in concepts. GCC and Clang are correct to reject this code, though the error message could be improved to state something to the effect of not allowing recursion in concepts.

BTW, there's no need for using recursion at all in this concept since we can use fold expressions. See https://godbolt.org/z/GTsbj9c4x for an example.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts and removed new issue labels Jun 4, 2022
@llvmbot
Copy link
Member

llvmbot commented Jun 4, 2022

@llvm/issue-subscribers-clang-frontend

@s2s-k
Copy link
Author

s2s-k commented Jun 4, 2022

BTW, there's no need for using recursion at all in this concept since we can use fold expressions. See https://godbolt.org/z/7YE483o5r for an example.

Yes, I know about fold-expressions. I noticed that it's possible to write this using a fold-expression after I originally wrote it.

I think this is an MSVC bug. Note that GCC rejects the code as well.

But still, that moment seemed weird to me. I don't know what the standard says about it. But from my point of view, this code looks logical and should compile.

@JoeLoser
Copy link
Member

JoeLoser commented Jun 4, 2022

But still, that moment seemed weird to me. I don't know what the standard says about it. But from my point of view, this code looks logical and should compile.

"Looks logical" and "should compile" don't always overlap. :)

This StackOverflow answer gives a good explanation of why it's not allowed. Additionally, instead of fold expressions, more generally speaking, concepts can be written in terms of type traits which can be recursive. See this StackOverflow answer for an example.

@s2s-k
Copy link
Author

s2s-k commented Jun 4, 2022

good explanation of why it's not allowed

The case they're talking about there is completely different than it's here. Recursion here is controlled with an expression in the ternary operator that will sometime become false. Consequently, infinity is out of the question. Also, the expression here is complete because the conditions are known at the time of compilation.

Also, you can catch me on the point that the ternary operator is not a constexpr and so the last IsAnyType in the ternary operator will not have any arguments when unpacking the template, so the compiler should have given an error... But that's not true either. Since it is a concept, it's possible to make "mistakes" in expression.

@s2s-k
Copy link
Author

s2s-k commented Jun 4, 2022

@JoeLoser btw, what do you think of this code?

MSVC: https://godbolt.org/z/YvcPov3Te
Clang: https://godbolt.org/z/h9K7d61v8

error: pack expansion used as argument for non-pack parameter of concept

This error looks very ridiculous. Will someone explain to me what it means?

@usx95 usx95 moved this to Triage Group 2 in C++ 20 in Clang Oct 14, 2022
@Endilll Endilll added c++20 and removed clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer labels Jan 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 20, 2024

@llvm/issue-subscribers-c-20

Author: None (s2s-k)

MSVC: https://godbolt.org/z/M9YKEq6df Clang: https://godbolt.org/z/rMM61PKKx

error: use of undeclared identifier 'IsAnyType'

I think the compiler gives this error for no reason, because "IsAnyType" has already been declared. Also, Microsoft's compiler compiles this code well.

@shafik
Copy link
Collaborator

shafik commented Jan 22, 2024

The original code is reject by clang/gcc/edg/MVSC: https://godbolt.org/z/rc49hEzcd now

The new example is rejected by gcc/clang and accepted by edg/MSVC: https://godbolt.org/z/jrs75nahY

Not sure who is correct, but we should probably open a second bug report for this since it looks like a different problem and does not look related to the original one.

cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Aug 14, 2024
Per [basic.scope], the locus of a concept is immediately
adfter the introduction of its name.

This let us provide better diagnostics for attempt
to define recursive concepts.

Note that recursive concepts are not supported per https://eel.is/c++draft/basic#scope.pdecl-note-3,
but there is no normative wording for that restriction.
This is a known defect introduced by [p1787r6](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1787r6.html).

Fixes llvm#55875
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Aug 14, 2024
Per [basic.scope], the locus of a concept is immediately
adfter the introduction of its name.

This let us provide better diagnostics for attempt
to define recursive concepts.

Note that recursive concepts are not supported per https://eel.is/c++draft/basic#scope.pdecl-note-3,
but there is no normative wording for that restriction.
This is a known defect introduced by [p1787r6](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1787r6.html).

Fixes llvm#55875
cor3ntin added a commit that referenced this issue Aug 14, 2024
Per [basic.scope], the locus of a concept is immediately after the
introduction of its name.

This let us provide better diagnostics for attempt to define recursive
concepts.

Note that recursive concepts are not supported per
https://eel.is/c++draft/basic#scope.pdecl-note-3, but there is no
normative wording for that restriction. This is a known defect
introduced by
[p1787r6](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1787r6.html).

Fixes #55875
bwendling pushed a commit to bwendling/llvm-project that referenced this issue Aug 15, 2024
Per [basic.scope], the locus of a concept is immediately after the
introduction of its name.

This let us provide better diagnostics for attempt to define recursive
concepts.

Note that recursive concepts are not supported per
https://eel.is/c++draft/basic#scope.pdecl-note-3, but there is no
normative wording for that restriction. This is a known defect
introduced by
[p1787r6](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1787r6.html).

Fixes llvm#55875
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants