Skip to content

[clang] Add a common definition of isPointerLikeType for lifetime analysis #117315

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 9 commits into from
Nov 28, 2024

Conversation

usx95
Copy link
Contributor

@usx95 usx95 commented Nov 22, 2024

Also checks for annotation for template specializations which sometimes may not have the annotation attached.

@usx95 usx95 requested a review from hokein November 22, 2024 11:03
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 22, 2024

@llvm/pr-subscribers-clang

Author: Utkarsh Saxena (usx95)

Changes

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

1 Files Affected:

  • (modified) clang/lib/Sema/CheckExprLifetime.cpp (+6-3)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 8886e5e307ddf8..64dc4794b6235a 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -253,9 +253,12 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
                                                   LocalVisitor Visit);
 
 template <typename T> static bool isRecordWithAttr(QualType Type) {
-  if (auto *RD = Type->getAsCXXRecordDecl())
-    return RD->hasAttr<T>();
-  return false;
+  auto *RD = Type->getAsCXXRecordDecl();
+  if (!RD)
+    return false;
+  if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
+    RD = CTSD->getSpecializedTemplate()->getTemplatedDecl();
+  return RD->hasAttr<T>();
 }
 
 // Decl::isInStdNamespace will return false for iterators in some STL

@usx95 usx95 requested a review from Endilll as a code owner November 28, 2024 09:03
@usx95 usx95 requested a review from hokein November 28, 2024 09:14
@@ -1763,6 +1763,18 @@ class Sema final : public SemaBase {
/// Add [[gsl::Pointer]] attributes for std:: types.
void inferGslPointerAttribute(TypedefNameDecl *TD);

template <typename T> static bool isRecordWithAttr(QualType Type) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd avoid expose this function if possible (as it seems only be used in CheckExprLifetime.cpp and isPointerLikeType). How about?

  • we just keep the isPointerLikeType declaration in Sema.h
  • define isPointerLikeType in CheckExprLifetime.cpp
  • the isRecordWithAttr can still be in CheckExprLifetime.cpp.

I think it is fine as CheckExprLifetime.cpp is a part of Sema library.

@usx95 usx95 requested a review from hokein November 28, 2024 10:47
Copy link

github-actions bot commented Nov 28, 2024

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

@@ -1763,6 +1763,8 @@ class Sema final : public SemaBase {
/// Add [[gsl::Pointer]] attributes for std:: types.
void inferGslPointerAttribute(TypedefNameDecl *TD);

static bool isPointerLikeType(QualType QT);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: please add some comments for this API. I think it is worth clarifying that it is only used for the lifetime analysis (to avoid misuse).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comments. Also moved to CheckExprLifetime.h.
I guess that is better to avoid wrong usages.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, nice! Moving to checkExprLifetime.h makes more sense.

@hokein
Copy link
Collaborator

hokein commented Nov 28, 2024

The PR description needs to update as well.

@usx95 usx95 changed the title [clang] Check specialization for annotation [clang] Add a common definition of isPointerLikeType for lifetime analysis Nov 28, 2024
@usx95 usx95 requested a review from hokein November 28, 2024 11:51
@usx95 usx95 merged commit 12ccb62 into llvm:main Nov 28, 2024
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 28, 2024

LLVM Buildbot has detected a new failure on builder libc-x86_64-debian-fullbuild-dbg-asan running on libc-x86_64-debian-fullbuild while building clang at step 4 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/171/builds/11264

Here is the relevant piece of the build log for the reference
Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure)
...
[==========] Running 4 tests from 1 test suite.
[ RUN      ] LlvmLibcHashTest.SanityCheck
[       OK ] LlvmLibcHashTest.SanityCheck (16 ms)
[ RUN      ] LlvmLibcHashTest.Avalanche
[       OK ] LlvmLibcHashTest.Avalanche (2176 ms)
[ RUN      ] LlvmLibcHashTest.UniformLSB
[       OK ] LlvmLibcHashTest.UniformLSB (202 ms)
[ RUN      ] LlvmLibcHashTest.UniformMSB
[       OK ] LlvmLibcHashTest.UniformMSB (148 us)
Ran 4 tests.  PASS: 4  FAIL: 0
command timed out: 1200 seconds without output running [b'python', b'../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py', b'--debug', b'--asan'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1258.812061
Step 8 (libc-unit-tests) failure: libc-unit-tests (failure)
...
[ RUN      ] LlvmLibcStrtoint64Test.InvalidBase
[       OK ] LlvmLibcStrtoint64Test.InvalidBase (13 us)
[ RUN      ] LlvmLibcStrtoint64Test.CleanBaseTenDecode
[       OK ] LlvmLibcStrtoint64Test.CleanBaseTenDecode (19 us)
[ RUN      ] LlvmLibcStrtoint64Test.MessyBaseTenDecode
[       OK ] LlvmLibcStrtoint64Test.MessyBaseTenDecode (9 us)
[ RUN      ] LlvmLibcStrtoint64Test.DecodeInOtherBases
[       OK ] LlvmLibcStrtoint64Test.DecodeInOtherBases (517 ms)
[ RUN      ] LlvmLibcStrtoint64Test.CleanBaseSixteenDecode
[       OK ] LlvmLibcStrtoint64Test.CleanBaseSixteenDecode (8 us)
[ RUN      ] LlvmLibcStrtoint64Test.MessyBaseSixteenDecode
[       OK ] LlvmLibcStrtoint64Test.MessyBaseSixteenDecode (3 us)
[ RUN      ] LlvmLibcStrtoint64Test.AutomaticBaseSelection
[       OK ] LlvmLibcStrtoint64Test.AutomaticBaseSelection (5 us)
[ RUN      ] LlvmLibcStrtouint64Test.InvalidBase
[       OK ] LlvmLibcStrtouint64Test.InvalidBase (2 us)
[ RUN      ] LlvmLibcStrtouint64Test.CleanBaseTenDecode
[       OK ] LlvmLibcStrtouint64Test.CleanBaseTenDecode (7 us)
[ RUN      ] LlvmLibcStrtouint64Test.MessyBaseTenDecode
[       OK ] LlvmLibcStrtouint64Test.MessyBaseTenDecode (7 us)
[ RUN      ] LlvmLibcStrtouint64Test.DecodeInOtherBases
[       OK ] LlvmLibcStrtouint64Test.DecodeInOtherBases (235 ms)
[ RUN      ] LlvmLibcStrtouint64Test.CleanBaseSixteenDecode
[       OK ] LlvmLibcStrtouint64Test.CleanBaseSixteenDecode (8 us)
[ RUN      ] LlvmLibcStrtouint64Test.MessyBaseSixteenDecode
[       OK ] LlvmLibcStrtouint64Test.MessyBaseSixteenDecode (4 us)
[ RUN      ] LlvmLibcStrtouint64Test.AutomaticBaseSelection
[       OK ] LlvmLibcStrtouint64Test.AutomaticBaseSelection (5 us)
Ran 14 tests.  PASS: 14  FAIL: 0
[1096/1098] Running unit test libc.test.src.time.nanosleep_test.__unit__
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcNanosleep.SmokeTest
[       OK ] LlvmLibcNanosleep.SmokeTest (46 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[1097/1098] Running unit test libc.test.src.__support.hash_test.__unit__
[==========] Running 4 tests from 1 test suite.
[ RUN      ] LlvmLibcHashTest.SanityCheck
[       OK ] LlvmLibcHashTest.SanityCheck (16 ms)
[ RUN      ] LlvmLibcHashTest.Avalanche
[       OK ] LlvmLibcHashTest.Avalanche (2176 ms)
[ RUN      ] LlvmLibcHashTest.UniformLSB
[       OK ] LlvmLibcHashTest.UniformLSB (202 ms)
[ RUN      ] LlvmLibcHashTest.UniformMSB
[       OK ] LlvmLibcHashTest.UniformMSB (148 us)
Ran 4 tests.  PASS: 4  FAIL: 0

command timed out: 1200 seconds without output running [b'python', b'../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py', b'--debug', b'--asan'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1258.812061

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