Check initialization is proceeding as expecting.

Don't blindly run through initialization; make sure things are going according to plan.
If not handle it and run destructors without crashing.
Return standard error codes from main.

include/cling/Interpreter/Interpreter.h

@@ -316,6 +316,10 @@ namespace cling {
 
     virtual ~Interpreter();
 
+    ///\brief Whether the Interpreter is setup and ready to be used.
+    ///
+    bool isValid() const;
+
     const InvocationOptions& getOptions() const { return m_Opts; }
     InvocationOptions& getOptions() { return m_Opts; }
 
@@ -615,6 +619,7 @@ namespace cling {
     void enableRawInput(bool raw = true) { m_RawInputEnabled = raw; }
 
     clang::CompilerInstance* getCI() const;
+    clang::CompilerInstance* getCIOrNull() const;
     clang::Sema& getSema() const;
 
     //FIXME: This must be in InterpreterCallbacks.

lib/Interpreter/IncrementalParser.cpp

@@ -180,11 +180,17 @@ namespace cling {
     m_Interpreter(interp),
     m_CI(CIFactory::createCI("", interp->getOptions(), llvmdir)),
     m_Consumer(nullptr), m_ModuleNo(0) {
-    assert(m_CI.get() && "CompilerInstance is (null)!");
+
+    if (!m_CI) {
+      cling::errs() << "Compiler instance could not be created.\n";
+      return;
+    }
 
     m_Consumer = dyn_cast<DeclCollector>(&m_CI->getSema().getASTConsumer());
-    assert(m_Consumer && "Expected ChainedConsumer!");
-
+    if (!m_Consumer) {
+      cling::errs() << "No AST consumer available.\n";
+      return;
+    }
 
     DiagnosticsEngine& Diag = m_CI->getDiagnostics();
     if (m_CI->getFrontendOpts().ProgramAction != frontend::ParseSyntaxOnly) {
@@ -204,7 +210,7 @@ namespace cling {
     initializeVirtualFile();
   }
 
-  void
+  bool
   IncrementalParser::Initialize(llvm::SmallVectorImpl<ParseResultTransaction>&
                                 result, bool isChildInterpreter) {
     m_TransactionPool.reset(new TransactionPool);
@@ -218,6 +224,7 @@ namespace cling {
 
     Transaction* CurT = beginTransaction(CO);
     Preprocessor& PP = m_CI->getPreprocessor();
+    DiagnosticsEngine& Diags = m_CI->getSema().getDiagnostics();
 
     // Pull in PCH.
     const std::string& PCHFileName
@@ -239,8 +246,8 @@ namespace cling {
     PP.EnterMainSourceFile();
 
     Sema* TheSema = &m_CI->getSema();
-    m_Parser.reset(new Parser(PP, *TheSema,
-                              false /*skipFuncBodies*/));
+    m_Parser.reset(new Parser(PP, *TheSema, false /*skipFuncBodies*/));
+
     // Initialize the parser after PP has entered the main source file.
     m_Parser->Initialize();
 
@@ -268,6 +275,13 @@ namespace cling {
     // been defined yet!
     ParseResultTransaction PRT = endTransaction(CurT);
     result.push_back(PRT);
+    return true;
+  }
+
+  bool IncrementalParser::isValid(bool initialized) const {
+    return m_CI && m_CI->hasFileManager() && m_Consumer
+           && !m_VirtualFileID.isInvalid()
+           && (!initialized || (m_TransactionPool && m_Parser));
   }
 
   const Transaction* IncrementalParser::getCurrentTransaction() const {
@@ -621,7 +635,8 @@ namespace cling {
   void IncrementalParser::initializeVirtualFile() {
     SourceManager& SM = getCI()->getSourceManager();
     m_VirtualFileID = SM.getMainFileID();
-    assert(!m_VirtualFileID.isInvalid() && "No VirtualFileID created?");
+    if (m_VirtualFileID.isInvalid())
+      cling::errs() << "VirtualFileID could not be created.\n";
   }
 
   IncrementalParser::ParseResultTransaction

lib/Interpreter/IncrementalParser.h

@@ -99,7 +99,13 @@ namespace cling {
     IncrementalParser(Interpreter* interp, const char* llvmdir);
     ~IncrementalParser();
 
-    void Initialize(llvm::SmallVectorImpl<ParseResultTransaction>& result,
+    ///\brief Whether the IncrementalParser is valid.
+    ///
+    ///\param[in] initialized - check if IncrementalParser has been initialized.
+    ///
+    bool isValid(bool initialized = true) const;
+
+    bool Initialize(llvm::SmallVectorImpl<ParseResultTransaction>& result,
                     bool isChildInterpreter);
     clang::CompilerInstance* getCI() const { return m_CI.get(); }
     clang::Parser* getParser() const { return m_Parser.get(); }

lib/Interpreter/Interpreter.cpp

@@ -159,6 +159,14 @@ namespace cling {
       == clang::frontend::ParseSyntaxOnly;
   }
 
+  bool Interpreter::isValid() const {
+    // Should we also check m_IncrParser->getFirstTransaction() ?
+    // not much can be done without it (its the initializing transaction)
+    return m_IncrParser && m_IncrParser->isValid() &&
+           m_DyLibManager && m_LookupHelper &&
+           (isInSyntaxOnlyMode() || m_Executor);
+  }
+  
   namespace internal { void symbol_requester(); }
 
   Interpreter::Interpreter(int argc, const char* const *argv,
@@ -172,6 +180,8 @@ namespace cling {
     m_LLVMContext.reset(new llvm::LLVMContext);
     m_DyLibManager.reset(new DynamicLibraryManager(getOptions()));
     m_IncrParser.reset(new IncrementalParser(this, llvmdir));
+    if (!m_IncrParser->isValid(false))
+      return;
 
     Sema& SemaRef = getSema();
     Preprocessor& PP = SemaRef.getPreprocessor();
@@ -182,9 +192,14 @@ namespace cling {
     m_LookupHelper.reset(new LookupHelper(new Parser(PP, SemaRef,
                                                      /*SkipFunctionBodies*/false,
                                                      /*isTemp*/true), this));
+    if (!m_LookupHelper)
+      return;
 
-    if (!isInSyntaxOnlyMode())
+    if (!isInSyntaxOnlyMode()) {
       m_Executor.reset(new IncrementalExecutor(SemaRef.Diags, *getCI()));
+      if (!m_Executor)
+        return;
+    }
 
     // Tell the diagnostic client that we are entering file parsing mode.
     DiagnosticConsumer& DClient = getCI()->getDiagnosticClient();
@@ -192,7 +207,14 @@ namespace cling {
 
     llvm::SmallVector<IncrementalParser::ParseResultTransaction, 2>
       IncrParserTransactions;
-    m_IncrParser->Initialize(IncrParserTransactions, parentInterp);
+    if (!m_IncrParser->Initialize(IncrParserTransactions, parentInterp)) {
+      // Initialization is not going well, but we still have to commit what
+      // we've been given. Don't clear the DiagnosticsConsumer so the caller
+      // can inspect any errors that have been generated.
+      for (auto&& I: IncrParserTransactions)
+        m_IncrParser->commitTransaction(I, false);
+      return;
+    }
 
     handleFrontendOptions();
 
@@ -255,23 +277,24 @@ namespace cling {
     Interpreter(argc, argv, llvmdir, noRuntime, &parentInterpreter) {
     // Do the "setup" of the connection between this interpreter and
     // its parent interpreter.
+    if (CompilerInstance* CI = getCIOrNull()) {
+      // The "bridge" between the interpreters.
+      ExternalInterpreterSource *myExternalSource =
+        new ExternalInterpreterSource(&parentInterpreter, this);
 
-    // The "bridge" between the interpreters.
-    ExternalInterpreterSource *myExternalSource =
-      new ExternalInterpreterSource(&parentInterpreter, this);
+      llvm::IntrusiveRefCntPtr <ExternalASTSource>
+        astContextExternalSource(myExternalSource);
 
-    llvm::IntrusiveRefCntPtr <ExternalASTSource>
-      astContextExternalSource(myExternalSource);
+      CI->getASTContext().setExternalSource(astContextExternalSource);
 
-    getCI()->getASTContext().setExternalSource(astContextExternalSource);
+      // Inform the Translation Unit Decl of I2 that it has to search somewhere
+      // else to find the declarations.
+      CI->getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage(true);
 
-    // Inform the Translation Unit Decl of I2 that it has to search somewhere
-    // else to find the declarations.
-    getCI()->getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage(true);
-
-    // Give my IncrementalExecutor a pointer to the Incremental executor of the
-    // parent Interpreter.
-    m_Executor->setExternalIncrementalExecutor(parentInterpreter.m_Executor.get());
+      // Give my IncrementalExecutor a pointer to the Incremental executor of the
+      // parent Interpreter.
+      m_Executor->setExternalIncrementalExecutor(parentInterpreter.m_Executor.get());
+    }
   }
 
   Interpreter::~Interpreter() {
@@ -279,7 +302,10 @@ namespace cling {
       m_Executor->shuttingDown();
     for (size_t i = 0, e = m_StoredStates.size(); i != e; ++i)
       delete m_StoredStates[i];
-    getCI()->getDiagnostics().getClient()->EndSourceFile();
+
+    if (CompilerInstance* CI = getCIOrNull())
+      getCI()->getDiagnostics().getClient()->EndSourceFile();
+
     // LookupHelper's ~Parser needs the PP from IncrParser's CI, so do this
     // first:
     m_LookupHelper.reset();
@@ -484,6 +510,10 @@ namespace cling {
     return m_IncrParser->getCI();
   }
 
+  CompilerInstance* Interpreter::getCIOrNull() const {
+    return m_IncrParser ? m_IncrParser->getCI() : nullptr;
+  }
+
   Sema& Interpreter::getSema() const {
     return getCI()->getSema();
   }
@@ -676,6 +706,8 @@ namespace cling {
     std::string llvmDir = parentResourceDir.str();
 
     Interpreter childInterpreter(*this, 1, &argV, llvmDir.c_str());
+    if (!childInterpreter.isValid())
+      return kFailure;
 
     auto childCI = childInterpreter.getCI();
     clang::Sema &childSemaRef = childCI->getSema();

tools/driver/cling.cpp

@@ -52,9 +52,17 @@ int main( int argc, char **argv ) {
 
   // Set up the interpreter
   cling::Interpreter interp(argc, argv);
-  if (interp.getOptions().Help) {
-    return 0;
+
+  if (!interp.isValid()) {
+    // Check if clang already diagnosed the error
+    clang::CompilerInstance* CI = interp.getCIOrNull();
+    if (CI && CI->getDiagnostics().getClient()->getNumErrors() == 0)
+      ::perror("Could not create Interpreter instance");
+    return EXIT_FAILURE;
   }
+  if (interp.getOptions().Help)
+    return EXIT_SUCCESS;
+
 
   clang::CompilerInstance* CI = interp.getCI();
   interp.AddIncludePath(".");
@@ -116,5 +124,6 @@ int main( int argc, char **argv ) {
   // Only for test/OutputRedirect.C, but shouldn't affect performance too much.
   ::fflush(stdout);
   ::fflush(stderr);
-  return ret;
+
+  return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }