Skip to content

[clang] Fix false warning on reinterpret_casting unknown template type #109430

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

Merged
merged 4 commits into from
Jan 20, 2025

Conversation

necto
Copy link
Contributor

@necto necto commented Sep 20, 2024

After 1595988
diag::warn_undefined_reinterpret_cast started raising on non-instantiated template functions without sufficient knowledge whether the reinterpret_cast is indeed UB.

This change fixes it.

CPP-5704

After 1595988
diag::warn_undefined_reinterpret_cast started raising on
non-instantiated template functions without sufficient knowledge whether
the reinterpret_cast is indeed UB.

This change fixes it.

CPP-5704
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 20, 2024

@llvm/pr-subscribers-clang

Author: Arseniy Zaostrovnykh (necto)

Changes

After 1595988
diag::warn_undefined_reinterpret_cast started raising on non-instantiated template functions without sufficient knowledge whether the reinterpret_cast is indeed UB.

This change fixes it.

CPP-5704


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

2 Files Affected:

  • (modified) clang/lib/Sema/SemaCast.cpp (+4)
  • (modified) clang/test/SemaCXX/reinterpret-cast.cpp (+62)
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 6ac6201843476b..dd65b301343e6c 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -2092,6 +2092,10 @@ void Sema::CheckCompatibleReinterpretCast(QualType SrcType, QualType DestType,
     }
   }
 
+  if (SrcTy->isTemplateTypeParmType() || DestTy->isTemplateTypeParmType()) {
+    return;
+  }
+
   Diag(Range.getBegin(), DiagID) << SrcType << DestType << Range;
 }
 
diff --git a/clang/test/SemaCXX/reinterpret-cast.cpp b/clang/test/SemaCXX/reinterpret-cast.cpp
index 45332fd15b5d4e..773aa8ee0e7894 100644
--- a/clang/test/SemaCXX/reinterpret-cast.cpp
+++ b/clang/test/SemaCXX/reinterpret-cast.cpp
@@ -302,3 +302,65 @@ void reinterpret_cast_allowlist () {
   (void)reinterpret_cast<unsigned char&>(b);
   (void)*reinterpret_cast<unsigned char*>(&b);
 }
+
+namespace templated {
+template <typename TARGETTYPE, typename UATYPE>
+void cast_uninstantiated() {
+  const UATYPE* data;
+  (void)*reinterpret_cast<const TARGETTYPE*>(data); // no warning
+}
+
+
+template <typename TARGETTYPE, typename UATYPE>
+void cast_instantiated_badly() {
+  const UATYPE* data;
+  (void)*reinterpret_cast<const TARGETTYPE*>(data); // expected-warning {{dereference of type 'const int *' that was reinterpret_cast from type 'const float *' has undefined behavior}}
+}
+
+template <typename TARGETTYPE, typename UATYPE>
+void cast_instantiated_well() {
+  const UATYPE* data;
+  (void)*reinterpret_cast<const TARGETTYPE*>(data); // no warning
+}
+
+template <typename TARGETTYPE>
+void cast_one_tmpl_arg_uninstantiated() {
+  const int* data;
+  (void)*reinterpret_cast<const TARGETTYPE*>(data); // no warning
+}
+
+template <typename TARGETTYPE>
+void cast_one_tmpl_arg_instantiated_badly() {
+  const float* data;
+  (void)*reinterpret_cast<const TARGETTYPE*>(data); // expected-warning {{dereference of type 'const int *' that was reinterpret_cast from type 'const float *' has undefined behavior}}
+}
+
+template <typename TARGETTYPE>
+void cast_one_tmpl_arg_instantiated_well() {
+  const float* data;
+  (void)*reinterpret_cast<const TARGETTYPE*>(data); // no warning
+}
+
+template <int size>
+void cast_nontype_template_true_positive_noninstantiated() {
+  const float *data;
+  const int arr[size];
+  (void)*reinterpret_cast<const int*>(data); // expected-warning {{dereference of type 'const int *' that was reinterpret_cast from type 'const float *' has undefined behavior}}
+}
+
+template <int size>
+void cast_nontype_template_true_negative_noninstantiated() {
+  const int data[size];
+  (void)*reinterpret_cast<const int*>(data); // no warning
+}
+
+void top() {
+  cast_instantiated_badly<int, float>();
+  // expected-note@-1 {{in instantiation of function template specialization 'templated::cast_instantiated_badly<int, float>' requested here}}
+  cast_instantiated_well<int, int>();
+  cast_one_tmpl_arg_instantiated_badly<int>();
+  // expected-note@-1 {{in instantiation of function template specialization 'templated::cast_one_tmpl_arg_instantiated_badly<int>' requested here}}
+  cast_one_tmpl_arg_instantiated_well<float>();
+}
+
+} // namespace templated

@cor3ntin cor3ntin requested a review from sdkrystian September 20, 2024 15:15
@@ -2092,6 +2092,10 @@ void Sema::CheckCompatibleReinterpretCast(QualType SrcType, QualType DestType,
}
}

if (SrcTy->isTemplateTypeParmType() || DestTy->isTemplateTypeParmType()) {
Copy link
Member

@sdkrystian sdkrystian Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably beif (SrcTy->isDependentType() || DestTy->isDependentType()). Otherwise we will continue to warn for the following:

template<typename T, typename U>
void f(T** x) 
{
    *reinterpret_cast<U**>(x);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point
Fixed in 541e0af

@necto necto requested a review from sdkrystian September 23, 2024 07:34
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Do you need me to merge that for you?

@necto
Copy link
Contributor Author

necto commented Jan 13, 2025

LGTM. Do you need me to merge that for you?

Yes, please

@cor3ntin
Copy link
Contributor

Hum, this actually need a release note, can you edit clang/docs/ReleaseNotes.rst? Thanks

@necto
Copy link
Contributor Author

necto commented Jan 15, 2025

Hum, this actually need a release note, can you edit clang/docs/ReleaseNotes.rst? Thanks

Added: e0db8fb

@necto
Copy link
Contributor Author

necto commented Jan 20, 2025

@cor3ntin is it good to go now?

@cor3ntin cor3ntin merged commit d049db8 into llvm:main Jan 20, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants