Template instantiation error recovery

[Purva-Chaudhari]

Summary

If error was encountered after template instantiation, the clang-repl interactive mode was aborted. The patch adds recovery support for template instantiation

Eg:

purva@purva-HP-Laptop-15-bs0xx:~/llvm-project/build$ bin/clang-repl
clang-repl> template<class T> T f() { return T(); }
clang-repl> auto ptu2 = f<float>(); err;
In file included from <<< inputs >>>:1:
input_line_1:1:25: error: C++ requires a type specifier for all declarations
auto ptu2 = f<float>(); err;
                        ^
clang-repl: /home/purva/llvm-project/clang/include/clang/Sema/Sema.h:9406: clang::Sema::GlobalEagerInstantiationScope::~GlobalEagerInstantiationScope(): Assertion `S.PendingInstantiations.empty() && "PendingInstantiations should be empty before it is discarded."' failed.
Aborted
 (core dumped)

[Purva-Chaudhari]

Shifted private member

[v.g.vassilev]

@Purva-Chaudhari, can you rebase this patch, seems that it is not buildable.

@rsmith, we need to do something similar in cling to handle pending template instantiations, I guess the question is if we can avoid instantiating the templates we don't need and still survive.

clang/lib/Interpreter/IncrementalParser.cpp

180

I suspect we should create this object earlier in this function. Can you export the diff with more context like suggested here https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

[v.g.vassilev]

In D125944#3544341, @v.g.vassilev wrote:

@Purva-Chaudhari, can you rebase this patch, seems that it is not buildable.

@rsmith, we need to do something similar in cling to handle pending template instantiations, I guess the question is if we can avoid instantiating the templates we don't need and still survive.

@rsmith, ping.

clang/test/Interpreter/execute.cpp

2 ↗

(On Diff #430579)

Can you move this test into a separate file which is dedicated for testing recovery of templates?

[Purva-Chaudhari]

Added new file for template test

[Purva-Chaudhari]

File name

[Purva-Chaudhari]

Updating

[Purva-Chaudhari]

add preamble to test file

[v.g.vassilev]

@Purva-Chaudhari can you rebase this patch?

[Purva-Chaudhari]

In D125944#3689022, @v.g.vassilev wrote:

@Purva-Chaudhari can you rebase this patch?

Yes. I realized I would have to rebase

clang/lib/Interpreter/IncrementalParser.cpp

180

Let me try it out

clang/test/Interpreter/execute.cpp

2 ↗

(On Diff #430579)

Yes updating the commit

[Purva-Chaudhari]

Rebase

[Purva-Chaudhari]

Rebase

[v.g.vassilev]

This looks reasonable to me but let's have another pair of eyes.

[aaron.ballman]

The precommit CI failure looks relevant:

******************** TEST 'Clang :: Interpreter/template-recovery.cpp' FAILED ********************

Script:

--

: 'RUN: at line 1';   /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);'             'auto r1 = printf("i = %d\n", i);' | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-DRIVER /var/lib/buildkite-agent/builds/llvm-project/clang/test/Interpreter/template-recovery.cpp

: 'RUN: at line 6';   cat /var/lib/buildkite-agent/builds/llvm-project/clang/test/Interpreter/template-recovery.cpp | /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang-repl | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/clang/test/Interpreter/template-recovery.cpp

--

Exit Code: 1



Command Output (stderr):

--

In file included from <<< inputs >>>:1:

input_line_8:1:25: error: a type specifier is required for all declarations

auto ptu2 = f<float>(); err;

                        ^

error: Parsing failed.

input_line_15:1:1: error: unknown type name 'quit'

quit

^

<<< inputs >>>:1:1: error: expected unqualified-id

error: Parsing failed.



--



********************

[Purva-Chaudhari]

In D125944#3689022, @v.g.vassilev wrote:

@Purva-Chaudhari can you rebase this patch?

In D125944#3821679, @aaron.ballman wrote:

The precommit CI failure looks relevant:

******************** TEST 'Clang :: Interpreter/template-recovery.cpp' FAILED ********************

Script:

--

: 'RUN: at line 1';   /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);'             'auto r1 = printf("i = %d\n", i);' | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-DRIVER /var/lib/buildkite-agent/builds/llvm-project/clang/test/Interpreter/template-recovery.cpp

: 'RUN: at line 6';   cat /var/lib/buildkite-agent/builds/llvm-project/clang/test/Interpreter/template-recovery.cpp | /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang-repl | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/clang/test/Interpreter/template-recovery.cpp

--

Exit Code: 1



Command Output (stderr):

--

In file included from <<< inputs >>>:1:

input_line_8:1:25: error: a type specifier is required for all declarations

auto ptu2 = f<float>(); err;

                        ^

error: Parsing failed.

input_line_15:1:1: error: unknown type name 'quit'

quit

^

<<< inputs >>>:1:1: error: expected unqualified-id

error: Parsing failed.



--



********************

Let me rebase and commit the test again

[Purva-Chaudhari]

rebase, test passing locally, in build stage 1

[Purva-Chaudhari]

fix test and clang format

[Purva-Chaudhari]

remove white space

[Purva-Chaudhari]

check

clang/include/clang/Sema/Sema.h

         SavedPendingLocalImplicitInstantiations;
   };
 
+  class PerformPendingInstantiationsRAII {
+  public:
+    PerformPendingInstantiationsRAII(Sema &S): S(S) {} ;
+
+    ~PerformPendingInstantiationsRAII() {
+      S.PerformPendingInstantiations();
+      assert(S.PendingInstantiations.empty() &&
+             "there shouldn't be any pending instantiations");
+    }
+  private:
+    Sema &S;
+  };
+
   /// A helper class for building up ExtParameterInfos.
   class ExtParameterInfoBuilder {
     SmallVector<FunctionProtoType::ExtParameterInfo, 16> Infos;

clang/lib/Interpreter/IncrementalParser.cpp

   llvm::CrashRecoveryContextCleanupRegistrar<Sema> CleanupSema(&S);
   Sema::GlobalEagerInstantiationScope GlobalInstantiations(S, /*Enabled=*/true);
   Sema::LocalEagerInstantiationScope LocalInstantiations(S);
+  Sema::PerformPendingInstantiationsRAII PerformPendingInstantiations(S);
 
   PTUs.emplace_back(PartialTranslationUnit());
   PartialTranslationUnit &LastPTU = PTUs.back();

clang/test/Interpreter/template-recovery.cpp

+// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
+// RUN:            'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// CHECK-DRIVER: i = 10
+// RUN: cat %s | clang-repl | FileCheck %s
+
+template<class T> T f() { return T(); }
+auto ptu2 = f<float>(); err;
+auto ptu2 = f<float>();
+
+extern "C" int printf(const char *, ...);
+int i = 10;
+auto r1 = printf("i = %d\n", i);
+// CHECK: i = 10
+quit