Skip to content

[clang] Don't emit the warn_dangling_lifetime_pointer diagnostic for the assignment case. #97408

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 3 commits into from
Jul 2, 2024

Conversation

hokein
Copy link
Collaborator

@hokein hokein commented Jul 2, 2024

The lifetime_pointer case is handled before the assignment case. In scenarios where we have the gsl::Pointer attribute, we may emit the -Wdangling-gsl warning for assignment cases. This means we cannot use -Wno-dangling-assignment to suppress the newly-added warning, this patch fixes it.

assignment case.

The lifetime_pointer case is handled before the assignment case. In some
scenario where we have the gsl::Pointer attr, we will end up with
emitting the `-Wdangling-gsl` warning for the assignment case. This
means we can not use `-Wno-dangling-assignment` to suppress the
newly-added warning.
@hokein hokein requested a review from usx95 July 2, 2024 12:17
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 2, 2024

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

The lifetime_pointer case is handled before the assignment case. In some scenario where we have the gsl::Pointer attr, we will end up with emitting the -Wdangling-gsl warning for the assignment case. This means we can not use -Wno-dangling-assignment to suppress the newly-added warning.


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

2 Files Affected:

  • (modified) clang/lib/Sema/CheckExprLifetime.cpp (+1-1)
  • (added) clang/test/SemaCXX/suppress-dangling-assignment.cpp (+15)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index fea0239f7bc36..03bf87a7cf5e6 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1023,7 +1023,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
         return false;
       }
 
-      if (IsGslPtrInitWithGslTempOwner && DiagLoc.isValid()) {
+      if (InitEntity && IsGslPtrInitWithGslTempOwner && DiagLoc.isValid()) {
         SemaRef.Diag(DiagLoc, diag::warn_dangling_lifetime_pointer)
             << DiagRange;
         return false;
diff --git a/clang/test/SemaCXX/suppress-dangling-assignment.cpp b/clang/test/SemaCXX/suppress-dangling-assignment.cpp
new file mode 100644
index 0000000000000..11625d3272b83
--- /dev/null
+++ b/clang/test/SemaCXX/suppress-dangling-assignment.cpp
@@ -0,0 +1,15 @@
+
+// RUN: %clang_cc1 -std=c++20 -verify -Wno-dangling-assignment %s
+// expected-no-diagnostics
+
+namespace std {
+// std::basic_string has a hard-coded gsl::owner attr.
+struct basic_string {
+  const char* c_str();
+};
+}  // namespace std
+
+void test(const char* a) {
+  // verify that the dangling-assignment diagnostic are suppressed. 
+  a = std::basic_string().c_str();
+}

@@ -1023,7 +1023,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
return false;
}

if (IsGslPtrInitWithGslTempOwner && DiagLoc.isValid()) {
if (InitEntity && IsGslPtrInitWithGslTempOwner && DiagLoc.isValid()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be simpler to just handle AEntity before:

if(AEntity) {
  assert(shouldLifetimeExtendThroughPath(Path) == PathLifetimeKind::NoExtend);
  assert(LK == LK_Extended);
  if (!pathContainsInit(path)) {
    SemaRef.Diag(DiagLoc, diag::warn_dangling_pointer_assignment)
              << AEntity->LHS << DiagRange;
  }
  return;
}
assert(InitEntity);
switch (LK) {
...
<remove assert(InitEntity*) checks>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@usx95 usx95 left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM.

@hokein hokein merged commit c3079ff into llvm:main Jul 2, 2024
4 of 6 checks passed
@hokein hokein deleted the always-dangling-assignment branch July 2, 2024 13:21
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
…the assignment case. (llvm#97408)

The `lifetime_pointer` case is handled before the assignment case. In
scenarios where we have the `gsl::Pointer` attribute, we may emit the
`-Wdangling-gsl` warning for assignment cases. This means we cannot use
`-Wno-dangling-assignment` to suppress the newly-added warning, this
patch fixes it.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
…the assignment case. (llvm#97408)

The `lifetime_pointer` case is handled before the assignment case. In
scenarios where we have the `gsl::Pointer` attribute, we may emit the
`-Wdangling-gsl` warning for assignment cases. This means we cannot use
`-Wno-dangling-assignment` to suppress the newly-added warning, this
patch fixes it.
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.

3 participants