[clang] 09d1f6e - [clang] Fix copy constructor of CompilerInvocation

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-04-14T09:13:35+02:00
New Revision: 09d1f6e6b74c9330d80c0346a271a43efbe0384d

URL: https://github.com/llvm/llvm-project/commit/09d1f6e6b74c9330d80c0346a271a43efbe0384d
DIFF: https://github.com/llvm/llvm-project/commit/09d1f6e6b74c9330d80c0346a271a43efbe0384d.diff

LOG: [clang] Fix copy constructor of CompilerInvocation

The `CompilerInvocationBase` class factors out members of `CompilerInvocation` that need special handling (initialization or copy constructor), so that `CompilerInvocation` can be implemented as a simple value object.

Currently, the `AnalyzerOpts` member of `CompilerInvocation` violates that setup. This patch extracts the member to `CompilerInvocationBase` and handles it in the copy constructor the same way other it handles other members.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D99568

Added: 
    

Modified: 
    clang/include/clang/Frontend/CompilerInvocation.h
    clang/lib/Frontend/CompilerInvocation.cpp
    clang/unittests/Frontend/CompilerInvocationTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Frontend/CompilerInvocation.h b/clang/include/clang/Frontend/CompilerInvocation.h
index 05b7769f20ef3..132a43a7def2a 100644
--- a/clang/include/clang/Frontend/CompilerInvocation.h
+++ b/clang/include/clang/Frontend/CompilerInvocation.h
@@ -78,6 +78,9 @@ class CompilerInvocationBase {
   /// Options controlling the preprocessor (aside from \#include handling).
   std::shared_ptr<PreprocessorOptions> PreprocessorOpts;
 
+  /// Options controlling the static analyzer.
+  AnalyzerOptionsRef AnalyzerOpts;
+
   CompilerInvocationBase();
   CompilerInvocationBase(const CompilerInvocationBase &X);
   CompilerInvocationBase &operator=(const CompilerInvocationBase &) = delete;
@@ -110,6 +113,8 @@ class CompilerInvocationBase {
   const PreprocessorOptions &getPreprocessorOpts() const {
     return *PreprocessorOpts;
   }
+
+  AnalyzerOptionsRef getAnalyzerOpts() const { return AnalyzerOpts; }
 };
 
 /// Helper class for holding the data necessary to invoke the compiler.
@@ -118,9 +123,6 @@ class CompilerInvocationBase {
 /// compiler, including data such as the include paths, the code generation
 /// options, the warning flags, and so on.
 class CompilerInvocation : public CompilerInvocationBase {
-  /// Options controlling the static analyzer.
-  AnalyzerOptionsRef AnalyzerOpts;
-
   MigratorOptions MigratorOpts;
 
   /// Options controlling IRgen and the backend.
@@ -139,8 +141,6 @@ class CompilerInvocation : public CompilerInvocationBase {
   PreprocessorOutputOptions PreprocessorOutputOpts;
 
 public:
-  CompilerInvocation() : AnalyzerOpts(new AnalyzerOptions()) {}
-
   /// @name Utility Methods
   /// @{
 
@@ -203,8 +203,6 @@ class CompilerInvocation : public CompilerInvocationBase {
   /// @name Option Subgroups
   /// @{
 
-  AnalyzerOptionsRef getAnalyzerOpts() const { return AnalyzerOpts; }
-
   MigratorOptions &getMigratorOpts() { return MigratorOpts; }
   const MigratorOptions &getMigratorOpts() const { return MigratorOpts; }
 

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index b18bc0b284ee5..b71143fdd0bb5 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -113,14 +113,16 @@ CompilerInvocationBase::CompilerInvocationBase()
     : LangOpts(new LangOptions()), TargetOpts(new TargetOptions()),
       DiagnosticOpts(new DiagnosticOptions()),
       HeaderSearchOpts(new HeaderSearchOptions()),
-      PreprocessorOpts(new PreprocessorOptions()) {}
+      PreprocessorOpts(new PreprocessorOptions()),
+      AnalyzerOpts(new AnalyzerOptions()) {}
 
 CompilerInvocationBase::CompilerInvocationBase(const CompilerInvocationBase &X)
     : LangOpts(new LangOptions(*X.getLangOpts())),
       TargetOpts(new TargetOptions(X.getTargetOpts())),
       DiagnosticOpts(new DiagnosticOptions(X.getDiagnosticOpts())),
       HeaderSearchOpts(new HeaderSearchOptions(X.getHeaderSearchOpts())),
-      PreprocessorOpts(new PreprocessorOptions(X.getPreprocessorOpts())) {}
+      PreprocessorOpts(new PreprocessorOptions(X.getPreprocessorOpts())),
+      AnalyzerOpts(new AnalyzerOptions(*X.getAnalyzerOpts())) {}
 
 CompilerInvocationBase::~CompilerInvocationBase() = default;
 

diff  --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp
index 066d02b02fa9d..5062a2e6cb8fe 100644
--- a/clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -97,6 +97,18 @@ TEST(ContainsN, Two) {
   ASSERT_THAT(Array, ContainsN(StrEq("x"), 2));
 }
 
+// Copy constructor performs a deep copy of reference-counted pointers.
+
+TEST(CompilerInvocationTest, DeepCopyConstructor) {
+  CompilerInvocation A;
+  A.getAnalyzerOpts()->Config["Key"] = "Old";
+
+  CompilerInvocation B(A);
+  B.getAnalyzerOpts()->Config["Key"] = "New";
+
+  ASSERT_EQ(A.getAnalyzerOpts()->Config["Key"], "Old");
+}
+
 // Boolean option with a keypath that defaults to true.
 // The only flag with a negative spelling can set the keypath to false.