-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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] Add tests from CWG156 to CWG1111 (dual-scope lookup for conversion-function-ids) #121654
Conversation
@llvm/pr-subscribers-clang Author: Vlad Serebrennikov (Endilll) ChangesThis patch adds test for CWG156. The most relevant piece of current wording is [basic.lookup.unqual]/5: Per resolution of CWG1111, additional lookup in the context of the entire postfix-expression, which originally was intended to cross-check lookup in the context of object-expression, was effectively turned into a fallback for it. Check out "Calling a conversion function" example in P1787R6 for step-by-step explanation of the current lookup mechanics for conversion functions. Clang rejects one of the well-formed examples, hence partial status. Clang is the only implementation which rejects it: https://godbolt.org/z/ohhbx8Mfs Full diff: https://github.com/llvm/llvm-project/pull/121654.diff 2 Files Affected:
diff --git a/clang/test/CXX/drs/cwg1xx.cpp b/clang/test/CXX/drs/cwg1xx.cpp
index 6aec8b65c91f12..eddad2e6a87b00 100644
--- a/clang/test/CXX/drs/cwg1xx.cpp
+++ b/clang/test/CXX/drs/cwg1xx.cpp
@@ -922,6 +922,49 @@ namespace cwg155 { // cwg155: dup 632
// expected-warning@-1 {{braces around scalar initializer}}
}
+namespace cwg156 { // cwg156: partial
+namespace ex1 {
+struct A {
+ operator int();
+} a;
+void foo() {
+ typedef int T;
+ a.operator T(); // T is found using unqualified lookup
+ // after qualified lookup in A fails.
+}
+} // namespace ex1
+
+namespace ex2 {
+struct A {
+ typedef int T;
+ operator T();
+};
+struct B : A {
+ operator T();
+} b;
+void foo() {
+ b.A::operator T(); // FIXME: qualified lookup should find T in A.
+ // expected-error@-1 {{unknown type name 'T'}}
+}
+} // namespace ex2
+
+namespace ex3 {
+template <class T1> struct A {
+ operator T1();
+};
+template <class T2> struct B : A<T2> {
+ operator T2();
+ void foo() {
+ // In both cases, during instantiation, qualified lookup for T2 wouldn't be able
+ // to find anything, so T2 has to be found by unqualified lookup.
+ // After that, 'operator T2()' is found in A<T2> by qualfied lookup.
+ T2 a = A<T2>::operator T2();
+ T2 b = ((A<T2> *)this)->operator T2();
+ }
+};
+} // namespace ex3
+} // namespace cwg156
+
// cwg158 is in cwg158.cpp
namespace cwg159 { // cwg159: 3.5
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index c069e155fd547c..bbdca49aad0533 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -981,7 +981,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/156.html">156</a></td>
<td>NAD</td>
<td>Name lookup for conversion functions</td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="partial" align="center">Partial</td>
</tr>
<tr class="open" id="157">
<td><a href="https://cplusplus.github.io/CWG/issues/157.html">157</a></td>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we just mark that as superseded and/or N/A.
The original requirements that lookup must find "the same thing" everywhere is gone so in effect we will never support the behavior that this issue is describing
clang/test/CXX/drs/cwg1xx.cpp
Outdated
b.A::operator T(); // FIXME: qualified lookup should find T in A. | ||
// expected-error@-1 {{unknown type name 'T'}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be #28181
Do we want to take a crack at it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is similar to that issue in a sense that T
needs to undergo a qualified lookup, but the lookup context is different (nested name specifier here, type of the object expression there).
Do we want to take a crack at it?
Maybe, but not in this PR.
To do that, we need to downgrade CWG1111 from being available in Clang 3.2 to "partial", and merge in at least the test that doesn't pass. Maybe we should do that anyway.
Note that it's resolved as NAD, so it's totally fine that the behavior described in the issue is not what we test. |
I'd say so, yes |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/10583 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/5080 Here is the relevant piece of the build log for the reference
|
This patch adds test from CWG156 to CWG1111 test, and downgrades the latter to partial availability. The most relevant piece of current wording is [basic.lookup.unqual]/5:
Per resolution of CWG1111, additional lookup in the context of the entire postfix-expression, which originally was intended to cross-check lookup in the context of object-expression, was effectively turned into a fallback for it.
Check out "Calling a conversion function" example in P1787R6 for step-by-step explanation of the current lookup mechanics for conversion functions.
Clang rejects one of the well-formed examples, hence partial status. Clang is the only implementation which rejects it: https://godbolt.org/z/ohhbx8Mfs