Add `IsRestrictQualifiedType` & `GetNonRestrictQualifiedType` functions #579

Vipul-Cariappa · 2025-05-15T09:27:09Z

Description

Example int *__restrict x

Fixes # (issue)

Helps fix a test case in cppyy.

Type of change

Please tick all options which are relevant.

Bug fix
New feature
Requires documentation updates

Testing

Added necessary tests.

Checklist

I have read the contribution guide recently

aaronj0 · 2025-05-15T09:36:16Z

lib/Interpreter/CppInterOp.cpp

+      NonRestrictType.removeLocalRestrict();
+      return NonRestrictType.getAsOpaquePtr();
+    }
+    return nullptr;


Should we not just return the type as-is if it is not restrict qualified?

Suggested change

return nullptr;

return type;

Otherwise, LGTM!

github-actions

clang-tidy made some suggestions

github-actions · 2025-05-15T09:37:36Z

lib/Interpreter/CppInterOp.cpp

@@ -1657,6 +1657,25 @@ namespace Cpp {
    return QT.getCanonicalType().getAsOpaquePtr();
  }

+  bool IsRestrictQualifiedType(TCppType_t type) {
+    if (!type)
+      return 0;


warning: converting integer literal to bool, use bool literal instead [modernize-use-bool-literals]

Suggested change

return 0;

return false;

github-actions · 2025-05-15T09:37:36Z

lib/Interpreter/CppInterOp.cpp

+
+  TCppType_t GetNonRestrictQualifiedType(TCppType_t type) {
+    if (!type)
+      return 0;


warning: use nullptr [modernize-use-nullptr]

Suggested change

return 0;

return nullptr;

github-actions · 2025-05-15T09:37:36Z

unittests/CppInterOp/TypeReflectionTest.cpp

@@ -605,3 +605,21 @@ TEST(TypeReflectionTest, IsFunctionPointerType) {
  EXPECT_FALSE(
      Cpp::IsFunctionPointerType(Cpp::GetVariableType(Cpp::GetNamed("i"))));
 }
+
+TEST(TypeReflectionTest, RestrictQualifiedType) {


warning: function 'TEST' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]

Suggested change

TEST(TypeReflectionTest, RestrictQualifiedType) {

TESTstatic (TypeReflectionTest, RestrictQualifiedType) {

codecov · 2025-05-15T09:41:07Z

Codecov Report

Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 76.46%. Comparing base (951775f) to head (13e2cbd).

Files with missing lines	Patch %	Lines
lib/Interpreter/CppInterOp.cpp	90.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #579      +/-   ##
==========================================
+ Coverage   76.43%   76.46%   +0.03%     
==========================================
  Files           9        9              
  Lines        3666     3676      +10     
==========================================
+ Hits         2802     2811       +9     
- Misses        864      865       +1

Files with missing lines	Coverage Δ
include/clang/Interpreter/CppInterOp.h	`96.29% <ø> (ø)`
lib/Interpreter/CppInterOp.cpp	`84.36% <90.00%> (+0.02%)`	⬆️

Files with missing lines	Coverage Δ
include/clang/Interpreter/CppInterOp.h	`96.29% <ø> (ø)`
lib/Interpreter/CppInterOp.cpp	`84.36% <90.00%> (+0.02%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

vgvassilev · 2025-05-15T10:14:28Z

include/clang/Interpreter/CppInterOp.h

@@ -546,6 +546,12 @@ namespace Cpp {
  /// out of it.
  CPPINTEROP_API TCppType_t GetCanonicalType(TCppType_t type);

+  /// Get non restrict qualified version of the given type
+  CPPINTEROP_API TCppType_t GetNonRestrictQualifiedType(TCppType_t type);


Suggested change

CPPINTEROP_API TCppType_t GetNonRestrictQualifiedType(TCppType_t type);

CPPINTEROP_API TCppType_t GetNonRestrictType(TCppType_t type);

vgvassilev · 2025-05-15T10:14:44Z

include/clang/Interpreter/CppInterOp.h

+  CPPINTEROP_API TCppType_t GetNonRestrictQualifiedType(TCppType_t type);
+
+  /// check if the type is restrict qualified i.e. __restrict
+  CPPINTEROP_API bool IsRestrictQualifiedType(TCppType_t type);


Suggested change

CPPINTEROP_API bool IsRestrictQualifiedType(TCppType_t type);

CPPINTEROP_API bool IsRestrictType(TCppType_t type);

mcbarton · 2025-05-15T11:29:54Z

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 76.41%. Comparing base (28ba16e) to head (48734c6).

Files with missing lines Patch % Lines
lib/Interpreter/CppInterOp.cpp 85.71% 2 Missing ⚠️
Additional details and impacted files
🚀 New features to boost your workflow:

Can you create a test/modify an existing one such that the tests cover 100% of the patch?

vgvassilev · 2025-05-23T15:29:22Z

lib/Interpreter/CppInterOp.cpp

+      NonRestrictType.removeLocalRestrict();
+      return NonRestrictType.getAsOpaquePtr();
+    }
+    return type;


Can we make codecov happy here?

I think we need a generic interface for the cvr types. Something like:

bool HasTypeQualifier(Quals::Restrict & Quals::Const & Quals::Volatile) { ... }
bool StripTypeQualifier(...) { ... return true;/if something was really stripped/}

Yes. That can be done.

enum Qual { Restrict = 0b01, Const = 0b010, Volatile = 0b0100 }; bool HasTypeQualifier(TCppType_t, enum Qual) { ... } TCppType RemoveTypeQualifier(TCppType_t, enum Qual) { ... }

The enum can be simply xored to combine different qualifiers.

@vgvassilev, Is this approach good?

Yes, makes sense. The enum should be const, volatile, restrict to follow the CVR terminology.

Look at #594

Vipul-Cariappa · 2025-05-26T09:33:50Z

The GitHub Actions are flaky today. Some are failing at the checkout step, or even before.

vgvassilev · 2025-05-26T09:35:05Z

The GitHub Actions are flaky today. Some are failing at the checkout step, or even before.

Yes: https://www.githubstatus.com/

Vipul-Cariappa requested review from vgvassilev and aaronj0 May 15, 2025 09:27

aaronj0 requested changes May 15, 2025

View reviewed changes

github-actions bot reviewed May 15, 2025

View reviewed changes

vgvassilev reviewed May 15, 2025

View reviewed changes

Vipul-Cariappa force-pushed the dev/restrict-type branch 2 times, most recently from 554d75b to 13e2cbd Compare May 23, 2025 09:02

vgvassilev reviewed May 23, 2025

View reviewed changes

Vipul-Cariappa added 2 commits May 26, 2025 09:43

Add IsRestrictQualifiedType & GetNonRestrictQualifiedType functions

480b7fa

changes according to code review

73fe9fc

Vipul-Cariappa force-pushed the dev/restrict-type branch from 13e2cbd to 73fe9fc Compare May 26, 2025 07:46

	TEST(TypeReflectionTest, RestrictQualifiedType) {
	TESTstatic (TypeReflectionTest, RestrictQualifiedType) {

	CPPINTEROP_API TCppType_t GetNonRestrictQualifiedType(TCppType_t type);
	CPPINTEROP_API TCppType_t GetNonRestrictType(TCppType_t type);

	CPPINTEROP_API bool IsRestrictQualifiedType(TCppType_t type);
	CPPINTEROP_API bool IsRestrictType(TCppType_t type);

Add IsRestrictQualifiedType & GetNonRestrictQualifiedType functions #579

Are you sure you want to change the base?

Add IsRestrictQualifiedType & GetNonRestrictQualifiedType functions #579

Uh oh!

Conversation

Vipul-Cariappa commented May 15, 2025

Description

Fixes # (issue)

Type of change

Testing

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot May 15, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot May 15, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot May 15, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcbarton commented May 15, 2025

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vgvassilev May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vipul-Cariappa commented May 26, 2025

Uh oh!

vgvassilev commented May 26, 2025

Uh oh!

Uh oh!

Add `IsRestrictQualifiedType` & `GetNonRestrictQualifiedType` functions #579

Add `IsRestrictQualifiedType` & `GetNonRestrictQualifiedType` functions #579

codecov bot commented May 15, 2025 •

edited

Loading

vgvassilev May 26, 2025 •

edited

Loading