This is an archive of the discontinued LLVM Phabricator instance.

Template instantiation error recovery
Needs ReviewPublic

Authored by Purva-Chaudhari on May 18 2022, 10:46 PM.

Details

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)

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2022, 10:46 PM
Purva-Chaudhari requested review of this revision.May 18 2022, 10:46 PM

Shifted private member

v.g.vassilev added a subscriber: rsmith.

@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

@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
3

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

Added new file for template test

add preamble to test file

@Purva-Chaudhari can you rebase this patch?

@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
3

Yes updating the commit

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

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 can you rebase this patch?

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 updated this revision to Diff 464159.EditedSep 30 2022, 12:12 AM

rebase, test passing locally, in build stage 1

fix test and clang format

remove white space