[flang] Fix build after 9e306ad4

[jansvoboda11]

No description provided.

[llvmbot]

@llvm/pr-subscribers-flang-driver

Author: Jan Svoboda (jansvoboda11)

Changes

---

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

8 Files Affected:

• (modified) flang/include/flang/Frontend/CompilerInstance.h (+1-1)
• (modified) flang/include/flang/Frontend/CompilerInvocation.h (+1-1)
• (modified) flang/include/flang/Frontend/TextDiagnosticPrinter.h (+2-2)
• (modified) flang/lib/Frontend/CompilerInstance.cpp (+2-3)
• (modified) flang/lib/Frontend/TextDiagnosticPrinter.cpp (+1-1)
• (modified) flang/tools/flang-driver/driver.cpp (+5-5)
• (modified) flang/tools/flang-driver/fc1_main.cpp (+2-3)
• (modified) flang/unittests/Frontend/CompilerInstanceTest.cpp (+3-2)

diff --git a/flang/include/flang/Frontend/CompilerInstance.h b/flang/include/flang/Frontend/CompilerInstance.h
index 4ad95c9df42d7..4234e13597cd7 100644
--- a/flang/include/flang/Frontend/CompilerInstance.h
+++ b/flang/include/flang/Frontend/CompilerInstance.h
@@ -347,7 +347,7 @@ class CompilerInstance {
   ///
   /// \return The new object on success, or null on failure.
   static clang::IntrusiveRefCntPtr<clang::DiagnosticsEngine>
-  createDiagnostics(clang::DiagnosticOptions *opts,
+  createDiagnostics(clang::DiagnosticOptions &opts,
                     clang::DiagnosticConsumer *client = nullptr,
                     bool shouldOwnClient = true);
   void createDiagnostics(clang::DiagnosticConsumer *client = nullptr,
diff --git a/flang/include/flang/Frontend/CompilerInvocation.h b/flang/include/flang/Frontend/CompilerInvocation.h
index d6ee1511cdb4b..06978029435b7 100644
--- a/flang/include/flang/Frontend/CompilerInvocation.h
+++ b/flang/include/flang/Frontend/CompilerInvocation.h
@@ -43,7 +43,7 @@ bool parseDiagnosticArgs(clang::DiagnosticOptions &opts,
 class CompilerInvocationBase {
 public:
   /// Options controlling the diagnostic engine.
-  llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> diagnosticOpts;
+  std::shared_ptr<clang::DiagnosticOptions> diagnosticOpts;
   /// Options for the preprocessor.
   std::shared_ptr<Fortran::frontend::PreprocessorOptions> preprocessorOpts;
 
diff --git a/flang/include/flang/Frontend/TextDiagnosticPrinter.h b/flang/include/flang/Frontend/TextDiagnosticPrinter.h
index 9c99a0c314351..4913713b6c365 100644
--- a/flang/include/flang/Frontend/TextDiagnosticPrinter.h
+++ b/flang/include/flang/Frontend/TextDiagnosticPrinter.h
@@ -37,13 +37,13 @@ class TextDiagnostic;
 
 class TextDiagnosticPrinter : public clang::DiagnosticConsumer {
   raw_ostream &os;
-  llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> diagOpts;
+  clang::DiagnosticOptions &diagOpts;
 
   /// A string to prefix to error messages.
   std::string prefix;
 
 public:
-  TextDiagnosticPrinter(raw_ostream &os, clang::DiagnosticOptions *diags);
+  TextDiagnosticPrinter(raw_ostream &os, clang::DiagnosticOptions &diags);
   ~TextDiagnosticPrinter() override;
 
   /// Set the diagnostic printer prefix string, which will be printed at the
diff --git a/flang/lib/Frontend/CompilerInstance.cpp b/flang/lib/Frontend/CompilerInstance.cpp
index cbd2c58eeeb47..2e0f91fb0521c 100644
--- a/flang/lib/Frontend/CompilerInstance.cpp
+++ b/flang/lib/Frontend/CompilerInstance.cpp
@@ -226,12 +226,11 @@ bool CompilerInstance::executeAction(FrontendAction &act) {
 
 void CompilerInstance::createDiagnostics(clang::DiagnosticConsumer *client,
                                          bool shouldOwnClient) {
-  diagnostics =
-      createDiagnostics(&getDiagnosticOpts(), client, shouldOwnClient);
+  diagnostics = createDiagnostics(getDiagnosticOpts(), client, shouldOwnClient);
 }
 
 clang::IntrusiveRefCntPtr<clang::DiagnosticsEngine>
-CompilerInstance::createDiagnostics(clang::DiagnosticOptions *opts,
+CompilerInstance::createDiagnostics(clang::DiagnosticOptions &opts,
                                     clang::DiagnosticConsumer *client,
                                     bool shouldOwnClient) {
   clang::IntrusiveRefCntPtr<clang::DiagnosticIDs> diagID(
diff --git a/flang/lib/Frontend/TextDiagnosticPrinter.cpp b/flang/lib/Frontend/TextDiagnosticPrinter.cpp
index 65626827af3b3..043440328b1f6 100644
--- a/flang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ b/flang/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -27,7 +27,7 @@
 using namespace Fortran::frontend;
 
 TextDiagnosticPrinter::TextDiagnosticPrinter(raw_ostream &diagOs,
-                                             clang::DiagnosticOptions *diags)
+                                             clang::DiagnosticOptions &diags)
     : os(diagOs), diagOpts(diags) {}
 
 TextDiagnosticPrinter::~TextDiagnosticPrinter() {}
diff --git a/flang/tools/flang-driver/driver.cpp b/flang/tools/flang-driver/driver.cpp
index ed52988feaa59..35cc2efc0ac01 100644
--- a/flang/tools/flang-driver/driver.cpp
+++ b/flang/tools/flang-driver/driver.cpp
@@ -43,9 +43,9 @@ std::string getExecutablePath(const char *argv0) {
 
 // This lets us create the DiagnosticsEngine with a properly-filled-out
 // DiagnosticOptions instance
-static clang::DiagnosticOptions *
+static std::unique_ptr<clang::DiagnosticOptions>
 createAndPopulateDiagOpts(llvm::ArrayRef<const char *> argv) {
-  auto *diagOpts = new clang::DiagnosticOptions;
+  auto diagOpts = std::make_unique<clang::DiagnosticOptions>();
 
   // Ignore missingArgCount and the return value of ParseDiagnosticArgs.
   // Any errors that would be diagnosed here will also be diagnosed later,
@@ -114,17 +114,17 @@ int main(int argc, const char **argv) {
   // Not in the frontend mode - continue in the compiler driver mode.
 
   // Create DiagnosticsEngine for the compiler driver
-  llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> diagOpts =
+  std::unique_ptr<clang::DiagnosticOptions> diagOpts =
       createAndPopulateDiagOpts(args);
   llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs> diagID(
       new clang::DiagnosticIDs());
   Fortran::frontend::TextDiagnosticPrinter *diagClient =
-      new Fortran::frontend::TextDiagnosticPrinter(llvm::errs(), &*diagOpts);
+      new Fortran::frontend::TextDiagnosticPrinter(llvm::errs(), *diagOpts);
 
   diagClient->setPrefix(
       std::string(llvm::sys::path::stem(getExecutablePath(args[0]))));
 
-  clang::DiagnosticsEngine diags(diagID, &*diagOpts, diagClient);
+  clang::DiagnosticsEngine diags(diagID, *diagOpts, diagClient);
 
   // Prepare the driver
   clang::driver::Driver theDriver(driverPath,
diff --git a/flang/tools/flang-driver/fc1_main.cpp b/flang/tools/flang-driver/fc1_main.cpp
index 49535275d084d..f2cd513d0028c 100644
--- a/flang/tools/flang-driver/fc1_main.cpp
+++ b/flang/tools/flang-driver/fc1_main.cpp
@@ -67,9 +67,8 @@ int fc1_main(llvm::ArrayRef<const char *> argv, const char *argv0) {
   // for parsing the arguments
   llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs> diagID(
       new clang::DiagnosticIDs());
-  llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> diagOpts =
-      new clang::DiagnosticOptions();
-  clang::DiagnosticsEngine diags(diagID, &*diagOpts, diagsBuffer);
+  clang::DiagnosticOptions diagOpts;
+  clang::DiagnosticsEngine diags(diagID, diagOpts, diagsBuffer);
   bool success = CompilerInvocation::createFromArgs(flang->getInvocation(),
                                                     argv, diags, argv0);
 
diff --git a/flang/unittests/Frontend/CompilerInstanceTest.cpp b/flang/unittests/Frontend/CompilerInstanceTest.cpp
index 3fe2f063e996a..bf62a64be229a 100644
--- a/flang/unittests/Frontend/CompilerInstanceTest.cpp
+++ b/flang/unittests/Frontend/CompilerInstanceTest.cpp
@@ -67,14 +67,15 @@ TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
   // 1. Set-up a basic DiagnosticConsumer
   std::string diagnosticOutput;
   llvm::raw_string_ostream diagnosticsOS(diagnosticOutput);
+  clang::DiagnosticOptions diagPrinterOpts;
   auto diagPrinter = std::make_unique<Fortran::frontend::TextDiagnosticPrinter>(
-      diagnosticsOS, new clang::DiagnosticOptions());
+      diagnosticsOS, *diagPrinterOpts;
 
   // 2. Create a CompilerInstance (to manage a DiagnosticEngine)
   CompilerInstance compInst;
 
   // 3. Set-up DiagnosticOptions
-  auto diagOpts = new clang::DiagnosticOptions();
+  clang::DiagnosticOptions diagOpts;
   // Tell the diagnostics engine to emit the diagnostic log to STDERR. This
   // ensures that a chained diagnostic consumer is created so that the test can
   // exercise the unowned diagnostic consumer in a chained consumer.

[kazutakahirata]

I'm testing this on top of #141131#. I'm getting:

flang/lib/Frontend/TextDiagnosticPrinter.cpp:84:17: error: member reference type 'clang::DiagnosticOptions' is not a pointer; did you mean to use '.'?
   84 |     if (diagOpts->ShowColors)
      |         ~~~~~~~~^~
      |                 .

[kazutakahirata]

@jansvoboda11 Here is the full list:

flang/lib/Frontend/TextDiagnosticPrinter.cpp:84:17: error: member reference type 'clang::DiagnosticOptions' is not a pointer; did you mean to use '.'?
   84 |     if (diagOpts->ShowColors)
      |         ~~~~~~~~^~
      |                 .
flang/lib/Frontend/TextDiagnosticPrinter.cpp:116:67: error: member reference type 'clang::DiagnosticOptions' is not a pointer; did you mean to use '.'?
  116 |                                                           diagOpts->ShowColors);
      |                                                           ~~~~~~~~^~
      |                                                                   .
flang/lib/Frontend/TextDiagnosticPrinter.cpp:120:15: error: member reference type 'clang::DiagnosticOptions' is not a pointer; did you mean to use '.'?
  120 |       diagOpts->ShowColors);
      |       ~~~~~~~~^~
      |               .
3 errors generated.

[jansvoboda11]

You clearly have a faster machine than I do 😅 I'm still building locally...

[kazutakahirata]

With your two commits in this PR, I get:

flang/unittests/Frontend/CompilerInstanceTest.cpp:72:22: error: indirection requires pointer operand ('clang::DiagnosticOptions' invalid)           
   72 |       diagnosticsOS, *diagPrinterOpts;                                                                                                                                                        
      |                      ^~~~~~~~~~~~~~~~                                                                                                                                                         
flang/unittests/Frontend/CompilerInstanceTest.cpp:72:38: error: expected ')'                                                                        
   72 |       diagnosticsOS, *diagPrinterOpts;                                                                                                                                                        
      |                                      ^                                                                                                                                                        
flang/unittests/Frontend/CompilerInstanceTest.cpp:71:80: note: to match this '('
   71 |   auto diagPrinter = std::make_unique<Fortran::frontend::TextDiagnosticPrinter>(
      |                                                                                ^
flang/unittests/Frontend/CompilerInstanceTest.cpp:82:11: error: member reference type 'clang::DiagnosticOptions' is not a pointer; did you mean to use '.'?
   82 |   diagOpts->DiagnosticLogFile = "-";
      |   ~~~~~~~~^~
      |           .
3 errors generated.

[kazutakahirata]

LGTM. With your 4 commits in this PR, I can now do ninja all test-depends, which includes flang and flang's unit tests. Thank you for fixing things quickly!

[jansvoboda11]

Thanks again!