[clang] [clang-cl] [Sema] Support MSVC non-const lvalue to user-defined temporary reference (PR #99833)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 21 17:48:54 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-driver
Author: Max Winkler (MaxEW707)
<details>
<summary>Changes</summary>
MSDN docs for reference: https://learn.microsoft.com/en-us/cpp/build/reference/zc-referencebinding-enforce-reference-binding-rules?view=msvc-170
The warning referenced in that MSDN article: https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4239?view=msvc-170
MSVC has an extension, that has been enabled by default until very recently when MSVC started their C++ conformance trend, that allows binding a user-defined type temporary to a non-const lvalue.
When trying to port over various large internal windows only tools this became hard as they rely heavily on this extension.
While I am not a fan of this extension changing our code to remove uses of it is quite hard do to how the code is structured.
The end goal is to longer rely on this extension but at this moment in time it is less bug prone to have clang support this extension and focus on removal sometime in the future after porting to clang.
>From the MSDN reference abive I did not implement the warning at level 4, C4239, since everyone disables said warning who requires this extension so I felt it wasn't worth the effort. Implement the MSVC extension is the main requirement.
Enable this new option with `/permissive` since `/permissive` enables this MSVC extension.
---
Full diff: https://github.com/llvm/llvm-project/pull/99833.diff
11 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+4)
- (modified) clang/include/clang/Basic/LangOptions.def (+1)
- (modified) clang/include/clang/Driver/Options.td (+12)
- (modified) clang/include/clang/Sema/Sema.h (+2)
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+5)
- (modified) clang/lib/Driver/ToolChains/MSVC.cpp (+1)
- (modified) clang/lib/Sema/SemaInit.cpp (+14-8)
- (modified) clang/lib/Sema/SemaOverload.cpp (+15-1)
- (modified) clang/test/Driver/cl-permissive.c (+7)
- (modified) clang/test/Driver/cl-zc.cpp (+2)
- (added) clang/test/SemaCXX/ms-reference-binding.cpp (+102)
``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4638b91b48f95..567effa83f845 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -486,6 +486,10 @@ New Compiler Flags
- ``-fpointer-tbaa`` enables emission of distinct type-based alias
analysis tags for incompatible pointers.
+- ``-fms-reference-binding`` and its clang-cl counterpart ``/Zc:referenceBinding``.
+ Implements the MSVC extension where expressions that bind a user-defined type temporary
+ to a non-const lvalue reference are allowed.
+
Deprecated Compiler Flags
-------------------------
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index a6f36b23f07dc..ac4cabb9a82c7 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -306,6 +306,7 @@ LANGOPT(HIPStdParInterposeAlloc, 1, 0, "Replace allocations / deallocations with
LANGOPT(OpenACC , 1, 0, "OpenACC Enabled")
LANGOPT(MSVCEnableStdcMacro , 1, 0, "Define __STDC__ with '-fms-compatibility'")
+LANGOPT(MSVCReferenceBinding , 1, 0, "Accept expressions that bind a non-const lvalue reference to a user-defined type temporary")
LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are unavailable")
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 8707e71f2a319..cd568c9f8f60f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3036,6 +3036,12 @@ def fms_extensions : Flag<["-"], "fms-extensions">, Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption]>,
HelpText<"Accept some non-standard constructs supported by the Microsoft compiler">,
MarshallingInfoFlag<LangOpts<"MicrosoftExt">>, ImpliedByAnyOf<[fms_compatibility.KeyPath]>;
+def fms_reference_binding : Flag<["-"], "fms-reference-binding">, Group<f_Group>,
+ Visibility<[ClangOption, CC1Option, CLOption]>,
+ HelpText<"Accept expressions that bind a non-const lvalue reference to a user-defined type temporary as supported by the Microsoft Compiler">,
+ MarshallingInfoFlag<LangOpts<"MSVCReferenceBinding">>;
+def fno_ms_reference_binding : Flag<["-"], "fno-ms-reference-binding">, Group<f_Group>,
+ Visibility<[ClangOption, CLOption]>;
defm asm_blocks : BoolFOption<"asm-blocks",
LangOpts<"AsmBlocks">, Default<fms_extensions.KeyPath>,
PosFlag<SetTrue, [], [ClangOption, CC1Option]>,
@@ -8467,6 +8473,12 @@ def _SLASH_Zc_wchar_t : CLFlag<"Zc:wchar_t">,
HelpText<"Enable C++ builtin type wchar_t (default)">;
def _SLASH_Zc_wchar_t_ : CLFlag<"Zc:wchar_t-">,
HelpText<"Disable C++ builtin type wchar_t">;
+def _SLASH_Zc_referenceBinding : CLFlag<"Zc:referenceBinding">,
+ HelpText<"Do not accept expressions that bind a non-const lvalue reference to a user-defined type temporary">,
+ Alias<fno_ms_reference_binding>;
+def _SLASH_Zc_referenceBinding_ : CLFlag<"Zc:referenceBinding-">,
+ HelpText<"Accept expressions that bind a non-const lvalue reference to a user-defined type temporary">,
+ Alias<fms_reference_binding>;
def _SLASH_Z7 : CLFlag<"Z7">, Alias<g_Flag>,
HelpText<"Enable CodeView debug information in object files">;
def _SLASH_ZH_MD5 : CLFlag<"ZH:MD5">,
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d638d31e050dc..34ea43c3fa2e2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10158,6 +10158,8 @@ class Sema final : public SemaBase {
CompareReferenceRelationship(SourceLocation Loc, QualType T1, QualType T2,
ReferenceConversions *Conv = nullptr);
+ bool AllowMSLValueReferenceBinding(Qualifiers Quals, QualType QT);
+
/// AddOverloadCandidate - Adds the given function to the set of
/// candidate functions, using the given function call arguments. If
/// @p SuppressUserConversions, then don't allow user-defined
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 71cdaa10416f4..f1cf00b03ae96 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7007,6 +7007,11 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
IsWindowsMSVC))
CmdArgs.push_back("-fms-extensions");
+ // -fno-ms-reference-binding is the default.
+ if (Args.hasFlag(options::OPT_fms_reference_binding,
+ options::OPT_fno_ms_reference_binding, false))
+ CmdArgs.push_back("-fms-reference-binding");
+
// -fms-compatibility=0 is default.
bool IsMSVCCompat = Args.hasFlag(
options::OPT_fms_compatibility, options::OPT_fno_ms_compatibility,
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index ca266e3e1d1d3..63ec69647c364 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -950,6 +950,7 @@ static void TranslatePermissive(Arg *A, llvm::opt::DerivedArgList &DAL,
const OptTable &Opts) {
DAL.AddFlagArg(A, Opts.getOption(options::OPT__SLASH_Zc_twoPhase_));
DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_operator_names));
+ DAL.AddFlagArg(A, Opts.getOption(options::OPT_fms_reference_binding));
}
static void TranslatePermissiveMinus(Arg *A, llvm::opt::DerivedArgList &DAL,
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index dc2ba039afe7f..0ccb2f285e766 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -5213,13 +5213,17 @@ static void TryReferenceInitializationCore(Sema &S,
Sequence.SetOverloadFailure(
InitializationSequence::FK_ReferenceInitOverloadFailed,
ConvOvlResult);
- else if (!InitCategory.isLValue())
- Sequence.SetFailed(
- T1Quals.isAddressSpaceSupersetOf(T2Quals)
- ? InitializationSequence::
- FK_NonConstLValueReferenceBindingToTemporary
- : InitializationSequence::FK_ReferenceInitDropsQualifiers);
- else {
+ else if (!InitCategory.isLValue()) {
+ if (T1Quals.isAddressSpaceSupersetOf(T2Quals)) {
+ if (!S.getLangOpts().MSVCReferenceBinding ||
+ !S.AllowMSLValueReferenceBinding(T1Quals, T1))
+ Sequence.SetFailed(InitializationSequence::
+ FK_NonConstLValueReferenceBindingToTemporary);
+ } else {
+ Sequence.SetFailed(
+ InitializationSequence::FK_ReferenceInitDropsQualifiers);
+ }
+ } else {
InitializationSequence::FailureKind FK;
switch (RefRelationship) {
case Sema::Ref_Compatible:
@@ -5245,7 +5249,9 @@ static void TryReferenceInitializationCore(Sema &S,
}
Sequence.SetFailed(FK);
}
- return;
+
+ if (Sequence.Failed())
+ return;
}
// - If the initializer expression
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index a8d250fbabfed..70802848e789a 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -4906,6 +4906,16 @@ Sema::CompareReferenceRelationship(SourceLocation Loc,
: Ref_Incompatible;
}
+bool Sema::AllowMSLValueReferenceBinding(Qualifiers Quals, QualType QT) {
+ if (Quals.hasVolatile())
+ return false;
+ if (Quals.hasConst())
+ return true;
+ if (QT->isBuiltinType() || QT->isArrayType())
+ return false;
+ return true;
+}
+
/// Look for a user-defined conversion to a value reference-compatible
/// with DeclType. Return true if something definite is found.
static bool
@@ -5138,7 +5148,11 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType,
// -- Otherwise, the reference shall be an lvalue reference to a
// non-volatile const type (i.e., cv1 shall be const), or the reference
// shall be an rvalue reference.
- if (!isRValRef && (!T1.isConstQualified() || T1.isVolatileQualified())) {
+ const bool CanBindLValueRef =
+ !S.getLangOpts().MSVCReferenceBinding
+ ? (T1.isConstQualified() && !T1.isVolatileQualified())
+ : S.AllowMSLValueReferenceBinding(T1.getQualifiers(), T1);
+ if (!isRValRef && !CanBindLValueRef) {
if (InitCategory.isRValue() && RefRelationship != Sema::Ref_Incompatible)
ICS.setBad(BadConversionSequence::lvalue_ref_to_rvalue, Init, DeclType);
return ICS;
diff --git a/clang/test/Driver/cl-permissive.c b/clang/test/Driver/cl-permissive.c
index d88746d3a5517..712bef71a43af 100644
--- a/clang/test/Driver/cl-permissive.c
+++ b/clang/test/Driver/cl-permissive.c
@@ -3,9 +3,11 @@
// RUN: %clang_cl /permissive -### -- %s 2>&1 | FileCheck -check-prefix=PERMISSIVE %s
// PERMISSIVE: "-fno-operator-names"
+// PERMISSIVE: "-fms-reference-binding"
// PERMISSIVE: "-fdelayed-template-parsing"
// RUN: %clang_cl /permissive- -### -- %s 2>&1 | FileCheck -check-prefix=PERMISSIVE-MINUS %s
// PERMISSIVE-MINUS-NOT: "-fno-operator-names"
+// PERMISSIVE-MINUS-NOT: "-fms-reference-binding"
// PERMISSIVE-MINUS-NOT: "-fdelayed-template-parsing"
// The switches set by permissive may then still be manually enabled or disabled
@@ -15,3 +17,8 @@
// RUN: %clang_cl /permissive- /Zc:twoPhase- -### -- %s 2>&1 | FileCheck -check-prefix=PERMISSIVE-MINUS-OVERWRITE %s
// PERMISSIVE-MINUS-OVERWRITE-NOT: "-fno-operator-names"
// PERMISSIVE-MINUS-OVERWRITE: "-fdelayed-template-parsing"
+
+// RUN: %clang_cl /permissive -fno-ms-reference-binding -### -- %s 2>&1 | FileCheck -check-prefix=PERMISSIVE-OVERWRITE-REF-BINDING %s
+// PERMISSIVE-OVERWRITE-REF-BINDING-NOT: "-fms-reference-binding"
+// RUN: %clang_cl /permissive- -fms-reference-binding -### -- %s 2>&1 | FileCheck -check-prefix=PERMISSIVE-MINUS-OVERWRITE-REF-BINDING %s
+// PERMISSIVE-MINUS-OVERWRITE-REF-BINDING: "-fms-reference-binding"
diff --git a/clang/test/Driver/cl-zc.cpp b/clang/test/Driver/cl-zc.cpp
index c7cf5b1b6525b..862d7a6e649c6 100644
--- a/clang/test/Driver/cl-zc.cpp
+++ b/clang/test/Driver/cl-zc.cpp
@@ -122,6 +122,8 @@
// RUN: %clang_cl -c -### /Zc:char8_t- -- %s 2>&1 | FileCheck -check-prefix CHECK-CHAR8_T_ %s
// CHECK-CHAR8_T_: "-fno-char8_t"
+// RUN: %clang_cl -c -### /Zc:referenceBinding- -- %s 2>&1 | FileCheck -check-prefix CHECK-REFERENCE_BINDING_ %s
+// CHECK-REFERENCE_BINDING_: "-fms-reference-binding"
// These never warn, but don't have an effect yet.
diff --git a/clang/test/SemaCXX/ms-reference-binding.cpp b/clang/test/SemaCXX/ms-reference-binding.cpp
new file mode 100644
index 0000000000000..44e4723fd4fa3
--- /dev/null
+++ b/clang/test/SemaCXX/ms-reference-binding.cpp
@@ -0,0 +1,102 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-reference-binding %s
+
+struct A {};
+struct B : A {};
+
+B fB();
+A fA();
+
+A&& fARvalueRef();
+A(&&fARvalueRefArray())[1];
+void fARef(A&) {}
+
+void fIntRef(int&) {} // expected-note{{candidate function not viable: expects an lvalue for 1st argument}}
+void fDoubleRef(double&) {} // expected-note{{candidate function not viable: expects an lvalue for 1st argument}}
+
+void fIntConstRef(const int&) {}
+void fDoubleConstRef(const double&) {}
+
+void fIntArray(int (&)[1]); // expected-note{{candidate function not viable: expects an lvalue for 1st argument}}
+void fIntConstArray(const int (&)[1]);
+
+namespace NS {
+ void fARef(A&) {}
+
+ void fIntRef(int&) {} // expected-note{{passing argument to parameter here}}
+ void fDoubleRef(double&) {} // expected-note{{passing argument to parameter here}}
+
+ void fIntConstRef(const int&) {}
+ void fDoubleConstRef(const double&) {}
+
+ A(&&fARvalueRefArray())[1];
+
+ void fIntArray(int (&)[1]); // expected-note{{passing argument to parameter here}}
+
+ void fIntConstArray(const int (&)[1]);
+}
+
+void test1() {
+ double& rd2 = 2.0; // expected-error{{non-const lvalue reference to type 'double' cannot bind to a temporary of type 'double'}}
+ int& i1 = 0; // expected-error{{non-const lvalue reference to type 'int' cannot bind to a temporary of type 'int'}}
+
+ fIntRef(0); // expected-error{{no matching function for call to 'fIntRef'}}
+ fDoubleRef(0.0); // expected-error{{no matching function for call to 'fDoubleRef'}}
+
+ NS::fIntRef(0); // expected-error{{non-const lvalue reference to type 'int' cannot bind to a temporary of type 'int'}}
+ NS::fDoubleRef(0.0); // expected-error{{non-const lvalue reference to type 'double' cannot bind to a temporary of type 'double'}}
+
+ int i2 = 2;
+ double& rd3 = i2; // expected-error{{non-const lvalue reference to type 'double' cannot bind to a value of unrelated type 'int'}}
+}
+
+void test2() {
+ fIntConstRef(0);
+ fDoubleConstRef(0.0);
+
+ NS::fIntConstRef(0);
+ NS::fDoubleConstRef(0.0);
+
+ int i = 0;
+ const int ci = 0;
+ volatile int vi = 0;
+ const volatile int cvi = 0;
+ bool b = true;
+
+ const volatile int &cvir1 = b ? ci : vi; // expected-error{{volatile lvalue reference to type 'const volatile int' cannot bind to a temporary of type 'int'}}
+
+ volatile int& vir1 = 0; // expected-error{{volatile lvalue reference to type 'volatile int' cannot bind to a temporary of type 'int'}}
+ const volatile int& cvir2 = 0; // expected-error{{volatile lvalue reference to type 'const volatile int' cannot bind to a temporary of type 'int'}}
+}
+
+void test3() {
+ A& a1 = A();
+
+ fARef(A());
+ fARef(static_cast<A&&>(a1));
+ fARef(B());
+
+ NS::fARef(A());
+ NS::fARef(static_cast<A&&>(a1));
+ NS::fARef(B());
+
+ A& a2 = fA();
+
+ A& a3 = fARvalueRef();
+
+ const A& rca = fB();
+ A& ra = fB();
+}
+
+void test4() {
+ A (&array1)[1] = fARvalueRefArray(); // expected-error{{non-const lvalue reference to type 'A[1]' cannot bind to a temporary of type 'A[1]'}}
+ const A (&array2)[1] = fARvalueRefArray();
+
+ A (&array3)[1] = NS::fARvalueRefArray(); // expected-error{{non-const lvalue reference to type 'A[1]' cannot bind to a temporary of type 'A[1]'}}
+ const A (&array4)[1] = NS::fARvalueRefArray();
+
+ fIntArray({ 1 }); // expected-error{{no matching function for call to 'fIntArray'}}
+ NS::fIntArray({ 1 }); // expected-error{{non-const lvalue reference to type 'int[1]' cannot bind to an initializer list temporary}}
+
+ fIntConstArray({ 1 });
+ NS::fIntConstArray({ 1 });
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/99833
More information about the cfe-commits
mailing list