[clang] Move CCC_OVERRIDE_OPTIONS implementation to Driver #85425

kastiglione · 2024-03-15T16:34:52Z

Move CCC_OVERRIDE_OPTIONS support to clangDriver so that it may be used outside of the clang driver binary.

The override functionality will be used in LLDB, to apply adjustments to ClangImporter flags. This will be useful as an escape hatch when there are issues that can be fixed by adding or removing clang flags.

The only thing changed is the name, from ApplyQAOverride to applyOverrideOptions.

Move CCC_OVERRIDE_OPTIONS support to clangDriver so that it may be used outside of the clang driver binary. The override functionality will be used in LLDB, to apply adjustments to ClangImporter flags. This will be useful as an escape hatch when there are issues that can be fixed by adding or removing clang flags.

llvmbot · 2024-03-15T16:35:21Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Dave Lee (kastiglione)

Changes

Move CCC_OVERRIDE_OPTIONS support to clangDriver so that it may be used outside
of the clang driver binary.

The override functionality will be used in LLDB, to apply adjustments to
ClangImporter flags. This will be useful as an escape hatch when there are
issues that can be fixed by adding or removing clang flags.

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

3 Files Affected:

(modified) clang/include/clang/Driver/Driver.h (+6-1)
(modified) clang/lib/Driver/Driver.cpp (+134)
(modified) clang/tools/driver/driver.cpp (+2-128)

diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index c4cab360bab3bb..d408907efb86b5 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -28,8 +28,8 @@
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/StringSaver.h"
 
-#include <list>
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -839,6 +839,11 @@ llvm::Error expandResponseFiles(SmallVectorImpl<const char *> &Args,
                                 bool ClangCLMode, llvm::BumpPtrAllocator &Alloc,
                                 llvm::vfs::FileSystem *FS = nullptr);
 
+void applyOverrideOptions(SmallVectorImpl<const char *> &Args,
+                          const char *OverrideOpts,
+                          std::set<std::string> &SavedStrings,
+                          raw_ostream *OS = nullptr);
+
 } // end namespace driver
 } // end namespace clang
 
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 190782a79a2456..43173ad3996fa3 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -87,6 +87,7 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/RISCVISAInfo.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
@@ -6677,3 +6678,136 @@ llvm::Error driver::expandResponseFiles(SmallVectorImpl<const char *> &Args,
 
   return llvm::Error::success();
 }
+
+namespace {
+
+const char *GetStableCStr(std::set<std::string> &SavedStrings, StringRef S) {
+  return SavedStrings.insert(std::string(S)).first->c_str();
+}
+
+/// ApplyOneQAOverride - Apply a list of edits to the input argument lists.
+///
+/// The input string is a space separated list of edits to perform,
+/// they are applied in order to the input argument lists. Edits
+/// should be one of the following forms:
+///
+///  '#': Silence information about the changes to the command line arguments.
+///
+///  '^': Add FOO as a new argument at the beginning of the command line.
+///
+///  '+': Add FOO as a new argument at the end of the command line.
+///
+///  's/XXX/YYY/': Substitute the regular expression XXX with YYY in the command
+///  line.
+///
+///  'xOPTION': Removes all instances of the literal argument OPTION.
+///
+///  'XOPTION': Removes all instances of the literal argument OPTION,
+///  and the following argument.
+///
+///  'Ox': Removes all flags matching 'O' or 'O[sz0-9]' and adds 'Ox'
+///  at the end of the command line.
+///
+/// \param OS - The stream to write edit information to.
+/// \param Args - The vector of command line arguments.
+/// \param Edit - The override command to perform.
+/// \param SavedStrings - Set to use for storing string representations.
+void applyOneOverrideOption(raw_ostream &OS,
+                            SmallVectorImpl<const char *> &Args, StringRef Edit,
+                            std::set<std::string> &SavedStrings) {
+  // This does not need to be efficient.
+
+  if (Edit[0] == '^') {
+    const char *Str = GetStableCStr(SavedStrings, Edit.substr(1));
+    OS << "### Adding argument " << Str << " at beginning\n";
+    Args.insert(Args.begin() + 1, Str);
+  } else if (Edit[0] == '+') {
+    const char *Str = GetStableCStr(SavedStrings, Edit.substr(1));
+    OS << "### Adding argument " << Str << " at end\n";
+    Args.push_back(Str);
+  } else if (Edit[0] == 's' && Edit[1] == '/' && Edit.ends_with("/") &&
+             Edit.slice(2, Edit.size() - 1).contains('/')) {
+    StringRef MatchPattern = Edit.substr(2).split('/').first;
+    StringRef ReplPattern = Edit.substr(2).split('/').second;
+    ReplPattern = ReplPattern.slice(0, ReplPattern.size() - 1);
+
+    for (unsigned i = 1, e = Args.size(); i != e; ++i) {
+      // Ignore end-of-line response file markers
+      if (Args[i] == nullptr)
+        continue;
+      std::string Repl = llvm::Regex(MatchPattern).sub(ReplPattern, Args[i]);
+
+      if (Repl != Args[i]) {
+        OS << "### Replacing '" << Args[i] << "' with '" << Repl << "'\n";
+        Args[i] = GetStableCStr(SavedStrings, Repl);
+      }
+    }
+  } else if (Edit[0] == 'x' || Edit[0] == 'X') {
+    auto Option = Edit.substr(1);
+    for (unsigned i = 1; i < Args.size();) {
+      if (Option == Args[i]) {
+        OS << "### Deleting argument " << Args[i] << '\n';
+        Args.erase(Args.begin() + i);
+        if (Edit[0] == 'X') {
+          if (i < Args.size()) {
+            OS << "### Deleting argument " << Args[i] << '\n';
+            Args.erase(Args.begin() + i);
+          } else
+            OS << "### Invalid X edit, end of command line!\n";
+        }
+      } else
+        ++i;
+    }
+  } else if (Edit[0] == 'O') {
+    for (unsigned i = 1; i < Args.size();) {
+      const char *A = Args[i];
+      // Ignore end-of-line response file markers
+      if (A == nullptr)
+        continue;
+      if (A[0] == '-' && A[1] == 'O' &&
+          (A[2] == '\0' || (A[3] == '\0' && (A[2] == 's' || A[2] == 'z' ||
+                                             ('0' <= A[2] && A[2] <= '9'))))) {
+        OS << "### Deleting argument " << Args[i] << '\n';
+        Args.erase(Args.begin() + i);
+      } else
+        ++i;
+    }
+    OS << "### Adding argument " << Edit << " at end\n";
+    Args.push_back(GetStableCStr(SavedStrings, '-' + Edit.str()));
+  } else {
+    OS << "### Unrecognized edit: " << Edit << "\n";
+  }
+}
+
+} // namespace
+
+/// ApplyQAOverride - Apply a space separated list of edits to the
+/// input argument lists. See ApplyOneQAOverride.
+void driver::applyOverrideOptions(SmallVectorImpl<const char *> &Args,
+                                  const char *OverrideStr,
+                                  std::set<std::string> &SavedStrings,
+                                  raw_ostream *OS) {
+  if (!OS)
+    OS = &llvm::nulls();
+
+  if (OverrideStr[0] == '#') {
+    ++OverrideStr;
+    OS = &llvm::nulls();
+  }
+
+  *OS << "### CCC_OVERRIDE_OPTIONS: " << OverrideStr << "\n";
+
+  // This does not need to be efficient.
+
+  const char *S = OverrideStr;
+  while (*S) {
+    const char *End = ::strchr(S, ' ');
+    if (!End)
+      End = S + strlen(S);
+    if (End != S)
+      applyOneOverrideOption(*OS, Args, std::string(S, End), SavedStrings);
+    S = End;
+    if (*S != '\0')
+      ++S;
+  }
+}
diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index 376025e3605be8..f7248fd5ad8b11 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -78,133 +78,6 @@ static const char *GetStableCStr(std::set<std::string> &SavedStrings,
   return SavedStrings.insert(std::string(S)).first->c_str();
 }
 
-/// ApplyOneQAOverride - Apply a list of edits to the input argument lists.
-///
-/// The input string is a space separated list of edits to perform,
-/// they are applied in order to the input argument lists. Edits
-/// should be one of the following forms:
-///
-///  '#': Silence information about the changes to the command line arguments.
-///
-///  '^': Add FOO as a new argument at the beginning of the command line.
-///
-///  '+': Add FOO as a new argument at the end of the command line.
-///
-///  's/XXX/YYY/': Substitute the regular expression XXX with YYY in the command
-///  line.
-///
-///  'xOPTION': Removes all instances of the literal argument OPTION.
-///
-///  'XOPTION': Removes all instances of the literal argument OPTION,
-///  and the following argument.
-///
-///  'Ox': Removes all flags matching 'O' or 'O[sz0-9]' and adds 'Ox'
-///  at the end of the command line.
-///
-/// \param OS - The stream to write edit information to.
-/// \param Args - The vector of command line arguments.
-/// \param Edit - The override command to perform.
-/// \param SavedStrings - Set to use for storing string representations.
-static void ApplyOneQAOverride(raw_ostream &OS,
-                               SmallVectorImpl<const char*> &Args,
-                               StringRef Edit,
-                               std::set<std::string> &SavedStrings) {
-  // This does not need to be efficient.
-
-  if (Edit[0] == '^') {
-    const char *Str =
-      GetStableCStr(SavedStrings, Edit.substr(1));
-    OS << "### Adding argument " << Str << " at beginning\n";
-    Args.insert(Args.begin() + 1, Str);
-  } else if (Edit[0] == '+') {
-    const char *Str =
-      GetStableCStr(SavedStrings, Edit.substr(1));
-    OS << "### Adding argument " << Str << " at end\n";
-    Args.push_back(Str);
-  } else if (Edit[0] == 's' && Edit[1] == '/' && Edit.ends_with("/") &&
-             Edit.slice(2, Edit.size() - 1).contains('/')) {
-    StringRef MatchPattern = Edit.substr(2).split('/').first;
-    StringRef ReplPattern = Edit.substr(2).split('/').second;
-    ReplPattern = ReplPattern.slice(0, ReplPattern.size()-1);
-
-    for (unsigned i = 1, e = Args.size(); i != e; ++i) {
-      // Ignore end-of-line response file markers
-      if (Args[i] == nullptr)
-        continue;
-      std::string Repl = llvm::Regex(MatchPattern).sub(ReplPattern, Args[i]);
-
-      if (Repl != Args[i]) {
-        OS << "### Replacing '" << Args[i] << "' with '" << Repl << "'\n";
-        Args[i] = GetStableCStr(SavedStrings, Repl);
-      }
-    }
-  } else if (Edit[0] == 'x' || Edit[0] == 'X') {
-    auto Option = Edit.substr(1);
-    for (unsigned i = 1; i < Args.size();) {
-      if (Option == Args[i]) {
-        OS << "### Deleting argument " << Args[i] << '\n';
-        Args.erase(Args.begin() + i);
-        if (Edit[0] == 'X') {
-          if (i < Args.size()) {
-            OS << "### Deleting argument " << Args[i] << '\n';
-            Args.erase(Args.begin() + i);
-          } else
-            OS << "### Invalid X edit, end of command line!\n";
-        }
-      } else
-        ++i;
-    }
-  } else if (Edit[0] == 'O') {
-    for (unsigned i = 1; i < Args.size();) {
-      const char *A = Args[i];
-      // Ignore end-of-line response file markers
-      if (A == nullptr)
-        continue;
-      if (A[0] == '-' && A[1] == 'O' &&
-          (A[2] == '\0' ||
-           (A[3] == '\0' && (A[2] == 's' || A[2] == 'z' ||
-                             ('0' <= A[2] && A[2] <= '9'))))) {
-        OS << "### Deleting argument " << Args[i] << '\n';
-        Args.erase(Args.begin() + i);
-      } else
-        ++i;
-    }
-    OS << "### Adding argument " << Edit << " at end\n";
-    Args.push_back(GetStableCStr(SavedStrings, '-' + Edit.str()));
-  } else {
-    OS << "### Unrecognized edit: " << Edit << "\n";
-  }
-}
-
-/// ApplyQAOverride - Apply a space separated list of edits to the
-/// input argument lists. See ApplyOneQAOverride.
-static void ApplyQAOverride(SmallVectorImpl<const char*> &Args,
-                            const char *OverrideStr,
-                            std::set<std::string> &SavedStrings) {
-  raw_ostream *OS = &llvm::errs();
-
-  if (OverrideStr[0] == '#') {
-    ++OverrideStr;
-    OS = &llvm::nulls();
-  }
-
-  *OS << "### CCC_OVERRIDE_OPTIONS: " << OverrideStr << "\n";
-
-  // This does not need to be efficient.
-
-  const char *S = OverrideStr;
-  while (*S) {
-    const char *End = ::strchr(S, ' ');
-    if (!End)
-      End = S + strlen(S);
-    if (End != S)
-      ApplyOneQAOverride(*OS, Args, std::string(S, End), SavedStrings);
-    S = End;
-    if (*S != '\0')
-      ++S;
-  }
-}
-
 extern int cc1_main(ArrayRef<const char *> Argv, const char *Argv0,
                     void *MainAddr);
 extern int cc1as_main(ArrayRef<const char *> Argv, const char *Argv0,
@@ -424,7 +297,8 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) {
   // scenes.
   if (const char *OverrideStr = ::getenv("CCC_OVERRIDE_OPTIONS")) {
     // FIXME: Driver shouldn't take extra initial argument.
-    ApplyQAOverride(Args, OverrideStr, SavedStrings);
+    driver::applyOverrideOptions(Args, OverrideStr, SavedStrings,
+                                 &llvm::errs());
   }
 
   std::string Path = GetExecutablePath(ToolContext.Path, CanonicalPrefixes);

clang/lib/Driver/Driver.cpp

MaskRay · 2024-03-15T18:11:47Z

clang/lib/Driver/Driver.cpp

+  return SavedStrings.insert(std::string(S)).first->c_str();
+}
+
+/// applyOneOverrideOption - Apply a list of edits to the input argument lists.


https://llvm.org/docs/CodingStandards.html

Don’t duplicate the documentation comment in the header file and in the implementation file.

Sorry, it's "Don’t duplicate function or class name at the beginning of the comment."

I will move it to the header file.

fixed these two and the include you pointed out.

MaskRay

two nits

clang/lib/Driver/Driver.cpp

MaskRay · 2024-03-15T18:28:05Z

clang/include/clang/Driver/Driver.h

+/// See applyOneOverrideOption.
+void applyOverrideOptions(SmallVectorImpl<const char *> &Args,
+                          const char *OverrideOpts,
+                          std::set<std::string> &SavedStrings,


Is a stable iteration order of SavedStrings needed? I haven't checked closely but it seems no. StringSet if more idiomatic.

thanks, replaced.

Move CCC_OVERRIDE_OPTIONS support to clangDriver so that it may be used outside of the clang driver binary. The override functionality will be used in LLDB, to apply adjustments to ClangImporter flags. This will be useful as an escape hatch when there are issues that can be fixed by adding or removing clang flags. The only thing changed is the name, from `ApplyQAOverride` to `applyOverrideOptions`. (cherry picked from commit 8d7ee46)

Move CCC_OVERRIDE_OPTIONS support to clangDriver so that it may be used outside of the clang driver binary. The override functionality will be used in LLDB, to apply adjustments to ClangImporter flags. This will be useful as an escape hatch when there are issues that can be fixed by adding or removing clang flags. The only thing changed is the name, from `ApplyQAOverride` to `applyOverrideOptions`. (cherry picked from commit 8d7ee46) (cherry-picked from commit f8ce350)

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Mar 15, 2024

kastiglione requested a review from adrian-prantl March 15, 2024 16:35

tbaederr reviewed Mar 15, 2024

View reviewed changes

clang/lib/Driver/Driver.cpp Outdated Show resolved Hide resolved

Fix function names in docstring

53b2904

adrian-prantl approved these changes Mar 15, 2024

View reviewed changes

MaskRay reviewed Mar 15, 2024

View reviewed changes

MaskRay approved these changes Mar 15, 2024

View reviewed changes

clang/lib/Driver/Driver.cpp Show resolved Hide resolved

kastiglione added 2 commits March 15, 2024 11:16

Remove unused include; Move docstring to header

f414c6e

Remove function name from beginning of docstring

76aec1b

MaskRay reviewed Mar 15, 2024

View reviewed changes

clang/lib/Driver/Driver.cpp Outdated Show resolved Hide resolved

MaskRay reviewed Mar 15, 2024

View reviewed changes

MaskRay approved these changes Mar 15, 2024

View reviewed changes

kastiglione added 2 commits March 15, 2024 14:49

Replace anon namespace with static functions

5965580

Replace std::set<std::string> with StringSet

da0efad

MaskRay approved these changes Mar 15, 2024

View reviewed changes

kastiglione merged commit 8d7ee46 into llvm:main Mar 15, 2024

kastiglione deleted the clang-Move-CCC_OVERRIDE_OPTIONS-implementation-to-Driver branch March 15, 2024 23:10

kastiglione mentioned this pull request May 2, 2024

[lldb] Add setting to override ClangImporter driver options swiftlang/llvm-project#8682

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang] Move CCC_OVERRIDE_OPTIONS implementation to Driver #85425

[clang] Move CCC_OVERRIDE_OPTIONS implementation to Driver #85425

Uh oh!

kastiglione commented Mar 15, 2024 •

edited

Loading

Uh oh!

llvmbot commented Mar 15, 2024 •

edited

Loading

Uh oh!

Uh oh!

MaskRay Mar 15, 2024

Uh oh!

MaskRay Mar 15, 2024

Uh oh!

kastiglione Mar 15, 2024

Uh oh!

kastiglione Mar 15, 2024

Uh oh!

MaskRay left a comment

Uh oh!

Uh oh!

Uh oh!

MaskRay Mar 15, 2024

Uh oh!

kastiglione Mar 15, 2024

Uh oh!

Uh oh!

[clang] Move CCC_OVERRIDE_OPTIONS implementation to Driver #85425

[clang] Move CCC_OVERRIDE_OPTIONS implementation to Driver #85425

Uh oh!

Conversation

kastiglione commented Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

MaskRay Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

MaskRay Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

kastiglione Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

kastiglione Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

MaskRay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MaskRay Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

kastiglione Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kastiglione commented Mar 15, 2024 •

edited

Loading

llvmbot commented Mar 15, 2024 •

edited

Loading