Add IsRestrictQualifiedType & GetNonRestrictQualifiedType functions#579
Add IsRestrictQualifiedType & GetNonRestrictQualifiedType functions#579Vipul-Cariappa wants to merge 2 commits intocompiler-research:mainfrom
IsRestrictQualifiedType & GetNonRestrictQualifiedType functions#579Conversation
| NonRestrictType.removeLocalRestrict(); | ||
| return NonRestrictType.getAsOpaquePtr(); | ||
| } | ||
| return nullptr; |
There was a problem hiding this comment.
Should we not just return the type as-is if it is not restrict qualified?
| return nullptr; | |
| return type; |
|
|
||
| bool IsRestrictQualifiedType(TCppType_t type) { | ||
| if (!type) | ||
| return 0; |
There was a problem hiding this comment.
warning: converting integer literal to bool, use bool literal instead [modernize-use-bool-literals]
| return 0; | |
| return false; |
|
|
||
| TCppType_t GetNonRestrictQualifiedType(TCppType_t type) { | ||
| if (!type) | ||
| return 0; |
There was a problem hiding this comment.
warning: use nullptr [modernize-use-nullptr]
| return 0; | |
| return nullptr; |
| Cpp::IsFunctionPointerType(Cpp::GetVariableType(Cpp::GetNamed("i")))); | ||
| } | ||
|
|
||
| TEST(TypeReflectionTest, RestrictQualifiedType) { |
There was a problem hiding this comment.
warning: function 'TEST' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]
| TEST(TypeReflectionTest, RestrictQualifiedType) { | |
| TESTstatic (TypeReflectionTest, RestrictQualifiedType) { |
Codecov ReportAttention: Patch coverage is
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
🚀 New features to boost your workflow:
|
| 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); |
There was a problem hiding this comment.
| CPPINTEROP_API TCppType_t GetNonRestrictQualifiedType(TCppType_t type); | |
| CPPINTEROP_API TCppType_t GetNonRestrictType(TCppType_t type); |
| 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); |
There was a problem hiding this comment.
| CPPINTEROP_API bool IsRestrictQualifiedType(TCppType_t type); | |
| CPPINTEROP_API bool IsRestrictType(TCppType_t type); |
Can you create a test/modify an existing one such that the tests cover 100% of the patch? |
554d75b to
13e2cbd
Compare
| NonRestrictType.removeLocalRestrict(); | ||
| return NonRestrictType.getAsOpaquePtr(); | ||
| } | ||
| return type; |
There was a problem hiding this comment.
Can we make codecov happy here?
There was a problem hiding this comment.
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/}
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yes, makes sense. The enum should be const, volatile, restrict to follow the CVR terminology.
13e2cbd to
73fe9fc
Compare
|
The GitHub Actions are flaky today. Some are failing at the checkout step, or even before. |
|
|
Closing in favour of #594 |
Description
Example
int *__restrict xFixes # (issue)
Helps fix a test case in cppyy.
Type of change
Please tick all options which are relevant.
Testing
Added necessary tests.
Checklist