Skip to content
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

Change type of DiagnosticHandlerTy #86504

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

Abhinkop
Copy link
Contributor

Changing type of DiagnosticHandlerTy due to adding -Wcast-function-type-mismatch to -Wextra group(#86131 (comment)). Changed the reference argument DiagnosticInfo to a pointer and edited the test cases failing due to this change. Added another small change where Gtest api was throwing an warning due varargs argument not being passed.

@llvmbot llvmbot added LTO Link time optimization (regular/full LTO or ThinLTO) llvm:support llvm:ir labels Mar 25, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 25, 2024

@llvm/pr-subscribers-llvm-support
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-lto

Author: Abhin P Jose (Abhinkop)

Changes

Changing type of DiagnosticHandlerTy due to adding -Wcast-function-type-mismatch to -Wextra group(#86131 (comment)). Changed the reference argument DiagnosticInfo to a pointer and edited the test cases failing due to this change. Added another small change where Gtest api was throwing an warning due varargs argument not being passed.


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

3 Files Affected:

  • (modified) llvm/include/llvm/IR/DiagnosticHandler.h (+2-2)
  • (modified) llvm/unittests/Linker/LinkModulesTest.cpp (+1-1)
  • (modified) llvm/unittests/Support/ThreadPool.cpp (+1-1)
diff --git a/llvm/include/llvm/IR/DiagnosticHandler.h b/llvm/include/llvm/IR/DiagnosticHandler.h
index db7d7444f75f05..aa1e4d14e82b61 100644
--- a/llvm/include/llvm/IR/DiagnosticHandler.h
+++ b/llvm/include/llvm/IR/DiagnosticHandler.h
@@ -28,7 +28,7 @@ struct DiagnosticHandler {
       : DiagnosticContext(DiagContext) {}
   virtual ~DiagnosticHandler() = default;
 
-  using DiagnosticHandlerTy = void (*)(const DiagnosticInfo &DI, void *Context);
+  using DiagnosticHandlerTy = void (*)(const DiagnosticInfo* DI, void *Context);
 
   /// DiagHandlerCallback is settable from the C API and base implementation
   /// of DiagnosticHandler will call it from handleDiagnostics(). Any derived
@@ -42,7 +42,7 @@ struct DiagnosticHandler {
   /// with a prefix based on the severity.
   virtual bool handleDiagnostics(const DiagnosticInfo &DI) {
     if (DiagHandlerCallback) {
-      DiagHandlerCallback(DI, DiagnosticContext);
+      DiagHandlerCallback(&DI, DiagnosticContext);
       return true;
     }
     return false;
diff --git a/llvm/unittests/Linker/LinkModulesTest.cpp b/llvm/unittests/Linker/LinkModulesTest.cpp
index 182ce73178c1d4..884e20e89c5c2f 100644
--- a/llvm/unittests/Linker/LinkModulesTest.cpp
+++ b/llvm/unittests/Linker/LinkModulesTest.cpp
@@ -72,7 +72,7 @@ class LinkModuleTest : public testing::Test {
   BasicBlock *ExitBB;
 };
 
-static void expectNoDiags(const DiagnosticInfo &DI, void *C) {
+static void expectNoDiags(const DiagnosticInfo *DI, void *C) {
   llvm_unreachable("expectNoDiags called!");
 }
 
diff --git a/llvm/unittests/Support/ThreadPool.cpp b/llvm/unittests/Support/ThreadPool.cpp
index d74c625d122950..381b4fc2a26b8e 100644
--- a/llvm/unittests/Support/ThreadPool.cpp
+++ b/llvm/unittests/Support/ThreadPool.cpp
@@ -126,7 +126,7 @@ using ThreadPoolImpls = ::testing::Types<
 #endif
     SingleThreadExecutor>;
 
-TYPED_TEST_SUITE(ThreadPoolTest, ThreadPoolImpls);
+TYPED_TEST_SUITE(ThreadPoolTest, ThreadPoolImpls, );
 
 #define CHECK_UNSUPPORTED()                                                    \
   do {                                                                         \

Copy link

✅ With the latest revision this PR passed the Python code formatter.

Copy link

github-actions bot commented Mar 25, 2024

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

@@ -28,7 +28,7 @@ struct DiagnosticHandler {
: DiagnosticContext(DiagContext) {}
virtual ~DiagnosticHandler() = default;

using DiagnosticHandlerTy = void (*)(const DiagnosticInfo &DI, void *Context);
using DiagnosticHandlerTy = void (*)(const DiagnosticInfo *DI, void *Context);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if this change will break some other projects.

Copy link
Contributor

@amy-kwan amy-kwan left a comment

Choose a reason for hiding this comment

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

If no one has any further comments, this LGTM and resolves the stage 2 bootstrap issue on the ppc64le-lld-multistage-test bot within my local testing.

@Abhinkop
Copy link
Contributor Author

Abhinkop commented Mar 26, 2024

@AaronBallman @amy-kwan Can we merge it? I'll watch the issue for a couple of days for any fallout breakages.

@amy-kwan
Copy link
Contributor

@AaronBallman @amy-kwan Can we merge it? I'll watch the issue for a couple of days for any fallout breakages.

Yes, I am OK if we merge this. LGTM.

@AaronBallman AaronBallman merged commit 44d037c into llvm:main Mar 26, 2024
4 checks passed
@Abhinkop Abhinkop deleted the fix_for_build_break_llvm branch March 26, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:ir llvm:support LTO Link time optimization (regular/full LTO or ThinLTO)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants