[clang] Improve bit-field in ref NTTP diagnostic

[bolshakov-a]

Prior to this, attempts to bind a bit-field to an NTTP of reference type produced an error because references to subobjects in NTTPs are disallowed. But C++20 allows references to subobjects in NTTPs generally (see P1907R1). Without this change, implementing P1907R1 would cause a bug allowing bit-fields to be bound to reference template arguments.

Extracted from https://reviews.llvm.org/D140996

[llvmbot]

@llvm/pr-subscribers-clang

Author: Andrey Ali Khan Bolshakov (bolshakov-a)

Changes

Prior to this, attempts to bind a bit-field to an NTTP of reference type produced an error because references to subobjects in NTTPs are disallowed. But C++20 allows references to subobjects in NTTPs generally (see P1907R1). Without this change, implementing P1907R1 would cause a bug allowing bit-fields to be bound to reference template arguments.

Extracted from https://reviews.llvm.org/D140996

---

Full diff: https://github.com/llvm/llvm-project/pull/71077.diff

3 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
• (modified) clang/lib/Sema/SemaOverload.cpp (+9)
• (modified) clang/test/CXX/drs/dr12xx.cpp (+1-1)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 474afc2fb99c1f0..94b836252eda571 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2251,6 +2251,8 @@ def warn_cxx17_compat_aggregate_init_paren_list : Warning<
 def err_reference_bind_to_bitfield : Error<
   "%select{non-const|volatile}0 reference cannot bind to "
   "bit-field%select{| %1}2">;
+def err_reference_bind_to_bitfield_in_cce : Error<
+  "reference cannot bind to bit-field in converted constant expression">;
 def err_reference_bind_to_vector_element : Error<
   "%select{non-const|volatile}0 reference cannot bind to vector element">;
 def err_reference_bind_to_matrix_element : Error<
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index d3d2dfed2ce0cc2..36390481e1ce6d4 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -5992,6 +5992,15 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From,
                                   /*InOverloadResolution=*/false,
                                   /*AllowObjCWritebackConversion=*/false,
                                   /*AllowExplicit=*/false);
+
+  // TryCopyInitialization returns incorrect info for attempts to bind reference
+  // to bit-field due to C++ [over.ics.ref]p4, so check it here.
+  if (From->refersToBitField() && T.getTypePtr()->isReferenceType()) {
+    return S.Diag(From->getBeginLoc(),
+                  diag::err_reference_bind_to_bitfield_in_cce)
+           << From->getSourceRange();
+  }
+
   StandardConversionSequence *SCS = nullptr;
   switch (ICS.getKind()) {
   case ImplicitConversionSequence::StandardConversion:
diff --git a/clang/test/CXX/drs/dr12xx.cpp b/clang/test/CXX/drs/dr12xx.cpp
index c23a515ba56cb99..ac28d99784d9c5a 100644
--- a/clang/test/CXX/drs/dr12xx.cpp
+++ b/clang/test/CXX/drs/dr12xx.cpp
@@ -138,7 +138,7 @@ namespace dr1295 { // dr1295: 4
 #if __cplusplus <= 201402L
   // expected-error@-2 {{does not refer to any declaration}} expected-note@-3 {{here}}
 #else
-  // expected-error@-4 {{refers to subobject}}
+  // expected-error@-4 {{bind to bit-field in converted constant expression}}
 #endif
 
 #if __cplusplus >= 201103L

[Endilll]

LGTM from DR testing perspective.

[cor3ntin]

Can you add a release note? Thanks.
Do we need more tests in other contexts where a converted constant expression is used?

[cor3ntin]

I think the wording that makes it not a constant expression is

A converted constant expression of type T is an expression, implicitly converted to type T, where the converted expression is a constant expression [...] where the reference binding (if any) binds directly. Bitfields don't bind directly.
It might be worth saying that in the comment

[bolshakov-a]

This citation is already present here. I've moved the new code closer to the last cited piece.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[bolshakov-a]

Can you add a release note?

Do you think this small diagnostic wording change is worth noting in relnotes?

Do we need more tests in other contexts where a converted constant expression is used?

Seems like constant expressions converted to references may currently only appear in non-type template arguments. Added a case into SemaTemplate/temp_arg_nontype_cxx20.cpp.

[shafik]

LGTM

[shafik]

Can you add a release note?

Do you think this small diagnostic wording change is worth noting in relnotes?

Yes, we have a Improvments to clang diagnostics section.

[bolshakov-a]

I want just note that it is probably not an improvement from the user's point of view, just a change of wording.

[cor3ntin]

Still looks good, thanks

[cor3ntin]

Merging at @bolshakov-a's request

[bolshakov-a]

Thanks!