Move CppInterOp.h into its own location; issue a warning to help users.#591
Merged
vgvassilev merged 5 commits intocompiler-research:mainfrom May 27, 2025
Merged
Conversation
vgvassilev
force-pushed
the
move-to-new-location
branch
4 times, most recently
from
a262a61 to
ef0abf9
Compare
Currently we have the `CppInterOp.h` in `clang/Interpreter/` which makes people think that it is a clang header file. This patch moves the header into its own folder to avoid confusion.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #591 +/- ##
==========================================
+ Coverage 76.43% 77.31% +0.88%
==========================================
Files 9 9
Lines 3666 3685 +19
==========================================
+ Hits 2802 2849 +47
+ Misses 864 836 -28
🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| void ReportInvokeEnd() const; | ||
|
|
||
| public: | ||
| Kind getKind() const { return m_Kind; } |
Contributor
There was a problem hiding this comment.
warning: function 'getKind' should be marked [[nodiscard]] [modernize-use-nodiscard]
Suggested change
| Kind getKind() const { return m_Kind; } | |
| [[nodiscard]] Kind getKind() const { return m_Kind; } |
Vipul-Cariappa
approved these changes
May 26, 2025
Collaborator
|
Thanks a lot for this PR. |
vgvassilev
changed the title
vgvassilev
force-pushed
the
move-to-new-location
branch
from
ef0abf9 to
8eb2abd
Compare
|
|
||
| public: | ||
| Kind getKind() const { return m_Kind; } | ||
| bool isValid() const { return getKind() != kUnknown; } |
Contributor
There was a problem hiding this comment.
warning: function 'isValid' should be marked [[nodiscard]] [modernize-use-nodiscard]
Suggested change
| bool isValid() const { return getKind() != kUnknown; } | |
| [[nodiscard]] bool isValid() const { return getKind() != kUnknown; } |
| public: | ||
| Kind getKind() const { return m_Kind; } | ||
| bool isValid() const { return getKind() != kUnknown; } | ||
| bool isInvalid() const { return !isValid(); } |
Contributor
There was a problem hiding this comment.
warning: function 'isInvalid' should be marked [[nodiscard]] [modernize-use-nodiscard]
Suggested change
| bool isInvalid() const { return !isValid(); } | |
| [[nodiscard]] bool isInvalid() const { return !isValid(); } |
vgvassilev
force-pushed
the
move-to-new-location
branch
from
8eb2abd to
6cee917
Compare
This would help us resolve headers such as gtest.h and reduce the clang-tidy compilation errors.
vgvassilev
force-pushed
the
move-to-new-location
branch
from
6cee917 to
54e04e6
Compare
| #include "clang-c/CXCppInterOp.h" | ||
| #include "clang-c/CXString.h" | ||
|
|
||
| using namespace clang; |
Contributor
There was a problem hiding this comment.
warning: expected namespace name [clang-diagnostic-error]
using namespace clang;
^
vgvassilev
force-pushed
the
move-to-new-location
branch
from
54e04e6 to
6be71ed
Compare
vgvassilev
force-pushed
the
move-to-new-location
branch
from
406b7d5 to
3cec1bf
Compare
vgvassilev
force-pushed
the
move-to-new-location
branch
from
3cec1bf to
3ef8b36
Compare
vgvassilev
force-pushed
the
move-to-new-location
branch
from
3ef8b36 to
dffab27
Compare
Contributor
Author
|
I cannot explain the windows failures. It might be that it has to do with some leftovers from the rename locally after the PR gets rebased on the bots... I'd prefer to merge this and debug later if necessary. |
vgvassilev
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently we have the
CppInterOp.hinclang/Interpreter/which makes people think that it is a clang header file. This patch moves the header into its own folder to avoid confusion.This patch also adds a warning for people to update their header files. That's technically not a breaking change and can land any time. The next patch will rename
libclangCppInterOpintolibCppInterOpwhich would be technically a breaking change and will be done in v2.0.This would be a good opportunity to fix stylistic changes without bloating the history such as the header preamble, indentation and such.