Skip to content

[clang] Fix LogDiagnosticPrinter.h and ClangTidyPlugin.cpp after 9e306ad4 #141131

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 22, 2025
Merged

[clang] Fix LogDiagnosticPrinter.h and ClangTidyPlugin.cpp after 9e306ad4 #141131

merged 2 commits into from
May 22, 2025

Conversation

@jansvoboda11
Copy link
Contributor

@jansvoboda11 jansvoboda11 commented May 22, 2025

No description provided.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra clang-tidy labels May 22, 2025
@llvmbot
Copy link
Member

llvmbot commented May 22, 2025 •
edited
Loading

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-tools-extra

Author: Jan Svoboda (jansvoboda11)

Changes

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

2 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp (+3-2)
  • (modified) clang/include/clang/Frontend/LogDiagnosticPrinter.h (-1)
diff --git a/clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp b/clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp
index 7911583db30e4..8c98ba7b9238a 100644
--- a/clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp
+++ b/clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp
@@ -40,9 +40,10 @@ class ClangTidyPluginAction : public PluginASTAction {
     // Create and set diagnostics engine
     auto *DiagConsumer =
         new ClangTidyDiagnosticConsumer(*Context, &Compiler.getDiagnostics());
+    auto DiagOpts = std::make_unique<DiagnosticOptions>();
     auto DiagEngine = std::make_unique<DiagnosticsEngine>(
-        new DiagnosticIDs, new DiagnosticOptions, DiagConsumer);
-    Context->setDiagnosticsEngine(DiagEngine.get());
+        new DiagnosticIDs, *DiagOpts, DiagConsumer);
+    Context->setDiagnosticsEngine(std::move(DiagOpts), DiagEngine.get());
 
     // Create the AST consumer.
     ClangTidyASTConsumerFactory Factory(*Context);
diff --git a/clang/include/clang/Frontend/LogDiagnosticPrinter.h b/clang/include/clang/Frontend/LogDiagnosticPrinter.h
index b43b0da13967a..9807dfa3aba1a 100644
--- a/clang/include/clang/Frontend/LogDiagnosticPrinter.h
+++ b/clang/include/clang/Frontend/LogDiagnosticPrinter.h
@@ -51,7 +51,6 @@ class LogDiagnosticPrinter : public DiagnosticConsumer {
   raw_ostream &OS;
   std::unique_ptr<raw_ostream> StreamOwner;
   const LangOptions *LangOpts;
-  DiagnosticOptions &DiagOpts;
 
   SourceLocation LastWarningLoc;
   FullSourceLoc LastLoc;

@llvmbot
Copy link
Member

llvmbot commented May 22, 2025

@llvm/pr-subscribers-clang-tidy

Author: Jan Svoboda (jansvoboda11)

Changes

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

2 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp (+3-2)
  • (modified) clang/include/clang/Frontend/LogDiagnosticPrinter.h (-1)
diff --git a/clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp b/clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp
index 7911583db30e4..8c98ba7b9238a 100644
--- a/clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp
+++ b/clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp
@@ -40,9 +40,10 @@ class ClangTidyPluginAction : public PluginASTAction {
     // Create and set diagnostics engine
     auto *DiagConsumer =
         new ClangTidyDiagnosticConsumer(*Context, &Compiler.getDiagnostics());
+    auto DiagOpts = std::make_unique<DiagnosticOptions>();
     auto DiagEngine = std::make_unique<DiagnosticsEngine>(
-        new DiagnosticIDs, new DiagnosticOptions, DiagConsumer);
-    Context->setDiagnosticsEngine(DiagEngine.get());
+        new DiagnosticIDs, *DiagOpts, DiagConsumer);
+    Context->setDiagnosticsEngine(std::move(DiagOpts), DiagEngine.get());
 
     // Create the AST consumer.
     ClangTidyASTConsumerFactory Factory(*Context);
diff --git a/clang/include/clang/Frontend/LogDiagnosticPrinter.h b/clang/include/clang/Frontend/LogDiagnosticPrinter.h
index b43b0da13967a..9807dfa3aba1a 100644
--- a/clang/include/clang/Frontend/LogDiagnosticPrinter.h
+++ b/clang/include/clang/Frontend/LogDiagnosticPrinter.h
@@ -51,7 +51,6 @@ class LogDiagnosticPrinter : public DiagnosticConsumer {
   raw_ostream &OS;
   std::unique_ptr<raw_ostream> StreamOwner;
   const LangOptions *LangOpts;
-  DiagnosticOptions &DiagOpts;
 
   SourceLocation LastWarningLoc;
   FullSourceLoc LastLoc;

@kazutakahirata
Copy link
Contributor

kazutakahirata commented May 22, 2025

@jansvoboda11 With this PR, I still see:

clang/lib/Frontend/LogDiagnosticPrinter.cpp:24:7: error: member initializer 'DiagOpts' does not name a non-static data member or base class
   24 |       DiagOpts(DiagOpts) {}
      |       ^~~~~~~~~~~~~~~~~~

Would you mind taking a look?

@kazutakahirata
Copy link
Contributor

kazutakahirata commented May 22, 2025

I still see:

/usr/local/google/home/kazu/dev/llvm/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:163:37: error: no member named 'Retain' in 'clang::DiagnosticOptions'
  163 |   static void retain(T *obj) { obj->Retain(); }
      |                                ~~~  ^

from flang/lib/Frontend/TextDiagnosticPrinter.cpp.

@jansvoboda11
Copy link
Contributor Author

jansvoboda11 commented May 22, 2025

@jansvoboda11 With this PR, I still see:

clang/lib/Frontend/LogDiagnosticPrinter.cpp:24:7: error: member initializer 'DiagOpts' does not name a non-static data member or base class
   24 |       DiagOpts(DiagOpts) {}
      |       ^~~~~~~~~~~~~~~~~~

Would you mind taking a look?

Fixed.

I still see:

/usr/local/google/home/kazu/dev/llvm/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:163:37: error: no member named 'Retain' in 'clang::DiagnosticOptions'
  163 |   static void retain(T *obj) { obj->Retain(); }
      |                                ~~~  ^

from flang/lib/Frontend/TextDiagnosticPrinter.cpp.

Working on a separate PR for Flang. Building to confirm I caught all issues.

kazutakahirata
kazutakahirata approved these changes May 22, 2025
View reviewed changes
Copy link
Contributor

@kazutakahirata kazutakahirata left a comment

Choose a reason for hiding this comment

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

LGTM. I think this PR fixes all non-flang components AFAICT. Thanks!

@jansvoboda11
Copy link
Contributor Author

jansvoboda11 commented May 22, 2025

Thank you too!

@jansvoboda11 jansvoboda11 merged commit 656d9ba into llvm:main May 22, 2025
6 of 10 checks passed
@jansvoboda11 jansvoboda11 deleted the clang-diag-opts-fixes branch May 22, 2025 20:39
@kazutakahirata kazutakahirata mentioned this pull request May 22, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kazutakahirata kazutakahirata kazutakahirata approved these changes

Assignees

No one assigned

Labels

clang Clang issues not falling into any other category clang-tidy clang-tools-extra

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jansvoboda11 @llvmbot @kazutakahirata