[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

This looks reasonable to me, but I'd wait for @aaron.ballman greenlight as he 
seems to have had more comments/ideas.




Comment at: clang/lib/Lex/Preprocessor.cpp:1000
+  std::vector toks;
+  while (1) {
+Token tok;

Hahnfeld wrote:
> aaron.ballman wrote:
> > v.g.vassilev wrote:
> > > aaron.ballman wrote:
> > > > Hahnfeld wrote:
> > > > > aaron.ballman wrote:
> > > > > > I'd prefer not to assume the token stream has an EOF token (perhaps 
> > > > > > the stream is one only being used to parse until the `eod` token 
> > > > > > instead), so if we can turn this into a non-infinite loop, that 
> > > > > > would make me more comfortable.
> > > > > I'm not sure I understand entirely. Do you want something like
> > > > > ```
> > > > > tok.isOneOf(tok::unknown, tok::eof, tok::eod)
> > > > > ```
> > > > > instead of `tok.is(tok::eof)`? Can this happen at the level of the 
> > > > > `Preprocessor`?
> > > > I was thinking something more along the lines of:
> > > > ```
> > > > if (Tokens) {
> > > >   for (Token Tok; !Tok.isOneOf(tok::eof, tok::eod); Lex(Tok))
> > > > Tokens->push_back(Tok);
> > > > }
> > > > ```
> > > > but I hadn't thought about `tok::unknown`; that might be a good one to 
> > > > also include given that clangd operates on partial sources.
> > > > 
> > > I was wondering if we could somehow merge this routine with 
> > > `Parser::SkipUntil` since they seem to be doing a very similar tasks.
> > That could perhaps end up looking reasonable (they do similar tasks aside 
> > from collecting the tokens that are being skipped). Do you need the 
> > interface to be on `Preprocessor` or `Parser` though (or does it not really 
> > matter for you)?
> > `tok.isOneOf(tok::unknown, tok::eof, tok::eod)`
> 
> I implemented this check, let me know if this looks reasonable. The code you 
> posted doesn't do what we need because we also want to lex if `Tokens` is 
> `nullptr`, so the hierarchy must be an `if` inside the loop.
> Given these additional token kinds, does `UntilEOF` still make sense or do we 
> want another name? Note that I'll leave `repl_input_end` to 
> https://reviews.llvm.org/D158415.
> 
> > I was wondering if we could somehow merge this routine with 
> > `Parser::SkipUntil` since they seem to be doing a very similar tasks.
> 
> I'm not sure this makes sense, given that `Parser::SkipUntil` requires some 
> knowledge about the structural input. At the very least, I'd prefer not to go 
> into that direction for this change.
I am not sure I understand the reasoning but I somewhat see that having 
Parser's `SkipUntil` be implemented with our new `Preprocessor::LexUntil...` 
would require a lot more work. How about adding a fixme note capturing this as 
a future possible refactoring?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158413/new/

https://reviews.llvm.org/D158413

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Lex/Preprocessor.cpp:1000
+  std::vector toks;
+  while (1) {
+Token tok;

aaron.ballman wrote:
> Hahnfeld wrote:
> > aaron.ballman wrote:
> > > I'd prefer not to assume the token stream has an EOF token (perhaps the 
> > > stream is one only being used to parse until the `eod` token instead), so 
> > > if we can turn this into a non-infinite loop, that would make me more 
> > > comfortable.
> > I'm not sure I understand entirely. Do you want something like
> > ```
> > tok.isOneOf(tok::unknown, tok::eof, tok::eod)
> > ```
> > instead of `tok.is(tok::eof)`? Can this happen at the level of the 
> > `Preprocessor`?
> I was thinking something more along the lines of:
> ```
> if (Tokens) {
>   for (Token Tok; !Tok.isOneOf(tok::eof, tok::eod); Lex(Tok))
> Tokens->push_back(Tok);
> }
> ```
> but I hadn't thought about `tok::unknown`; that might be a good one to also 
> include given that clangd operates on partial sources.
> 
I was wondering if we could somehow merge this routine with `Parser::SkipUntil` 
since they seem to be doing a very similar tasks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158413/new/

https://reviews.llvm.org/D158413

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158413: [Lex] Introduce Preprocessor::LexAll()

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Lex/Preprocessor.cpp:998
 
+std::vector Preprocessor::LexAll() {
+  std::vector toks;

Shouldn't we take the results as a `LexAll(std::vector )`?

Perhaps we should rename this interface to `LexTokensUntilEOF`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158413/new/

https://reviews.llvm.org/D158413

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158415: [Lex] Handle repl_input_end in Preprocessor::LexAll()

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

@Hahnfeld, let's move forward and rely on a post-commit review here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158415/new/

https://reviews.llvm.org/D158415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D159339#4642147 , @daltenty wrote:

> FYI: I tried to reproduce this on `powerpc64le-linux-gnu` at `-02` and didn't 
> see it at this point, though I get a whole lot of other uninitialized reads, 
> so there's definitely some general problems in this space in the LLVM codebase

Are these also present in llvm16?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159115: [clang-repl] Adapt to the recent dylib-related changes in ORC.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a subscriber: lkail.
v.g.vassilev added a comment.

In D159115#4642111 , @tuliom wrote:

> In D159115#4641834 , @mgorny wrote:
>
>> Is that actually a regression, or merely the test wasn't checking it before?
>
> I can't tell because the test didn't exist before.
>
>> My educated guess would be that the code in the test redefines `printf` 
>> without the header mangling, so it finds and compares the "non-IEEE" symbol 
>> against the "IEEE" symbol that's exposed to the test case via the headers.
>
> I agree with you.

Would that be a problem with the RuntimeDyld part of the JIT on ppc64le? Maybe 
@lkail could help us sort it out?

cc: @lhames


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159115/new/

https://reviews.llvm.org/D159115

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159167: [clang-repl][Orc] Export executable symbols in ClangReplInterpreterExceptionTests

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159167/new/

https://reviews.llvm.org/D159167

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a subscriber: Hahnfeld.
v.g.vassilev added a comment.

The issue goes away for non-CXXMethodDecls but still persists in the case where 
we process:

  CXXDestructorDecl 0x9ab0f48 

 col:11 implicit used ~shared_ptr 'void () noexcept' inline default
  `-CompoundStmt 0x9ac3ca0 

Here is a bt:

  #0  llvm::Value::getPointerAlignment (this=0x9dcbed8, DL=...) at 
/home/vvassilev/workspace/sources/llvm-project/llvm/lib/IR/Value.cpp:923
  #1  0x01b32b52 in 
clang::CodeGen::CodeGenModule::SetLLVMFunctionAttributesForDefinition 
(this=0x885e3c0, D=0x9ab0f48, F=0x9dcbed8)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:2389
  #2  0x01d40899 in clang::CodeGen::CodeGenModule::codegenCXXStructor 
(this=0x885e3c0, GD=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CGCXX.cpp:211
  #3  0x01bbffd5 in (anonymous 
namespace)::ItaniumCXXABI::emitCXXStructor (this=0x885f2f0, GD=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/ItaniumCXXABI.cpp:4453
  #4  0x01b36ffb in clang::CodeGen::CodeGenModule::EmitGlobalDefinition 
(this=this@entry=0x885e3c0, GD=..., GV=GV@entry=0x9dcbed8)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:3893
  #5  0x01b2a9a0 in clang::CodeGen::CodeGenModule::EmitDeferred 
(this=this@entry=0x885e3c0)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:3050
  #6  0x01b2a9cc in clang::CodeGen::CodeGenModule::EmitDeferred 
(this=this@entry=0x885e3c0)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:3056
  #7  0x01b285bb in clang::CodeGen::CodeGenModule::Release 
(this=0x885e3c0)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:759
  #8  0x01882908 in (anonymous 
namespace)::CodeGeneratorImpl::HandleTranslationUnit (this=0x87c6430, Ctx=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/ModuleBuilder.cpp:287
  #9  0x0186e3a6 in clang::BackendConsumer::HandleTranslationUnit 
(this=0x87c6080, C=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenAction.cpp:311
  #10 0x00f7bdc7 in clang::IncrementalParser::ParseOrWrapTopLevelDecl 
(this=this@entry=0x87603c0)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/Interpreter/IncrementalParser.cpp:303
  #11 0x00f7c760 in clang::IncrementalParser::Parse (this=0x87603c0, 
input=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/Interpreter/IncrementalParser.cpp:342
  #12 0x00f70c79 in clang::Interpreter::Parse (this=0x875d170, Code=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/Interpreter/Interpreter.cpp:360
  #13 0x00e28b11 in Cpp::Interpreter::Parse (this=0x86e9cf0, Code=...)
  at 
/home/vvassilev/workspace/builds/scratch/cppyy/InterOp/include/clang/Interpreter/CppInterOpInterpreter.h:172
  #14 0x00e28b84 in Cpp::Interpreter::declare (this=0x86e9cf0, 
  input="\n#include \n\ntemplate\nclass 
derived_shared_ptr : public std::shared_ptr {};\ntemplate\n  
  class derived_unique_ptr : public std::unique_ptr {};\n\n   "..., PTU=0x0)

I am staring at the `Value::getPointerAlignment(const DataLayout )` and 
cannot find out what might be wrong. One particularity of the clang-repl based 
workflows is that it calls multiple times `StartModule`, which creates a new 
`llvm::Module` per partial translation unit.

cc: @Hahnfeld


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159115: [clang-repl] Adapt to the recent dylib-related changes in ORC.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D159115#4638323 , @mgorny wrote:

> Changing the type from `unsigned long long` to `uintptr_t` fix the test for 
> me.

Ah! Nice catch! Can you commit the fix?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159115/new/

https://reviews.llvm.org/D159115

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158415: [Lex] Handle repl_input_end in Preprocessor::LexAll()

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

Looks reasonable to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158415/new/

https://reviews.llvm.org/D158415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

[Vassil Vassilev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG92246a9be0ba: [CodeGen] First check the kind and then the 
llvm::Function properties. (authored by v.g.vassilev).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2401,7 +2401,7 @@
   // functions. If the current target's C++ ABI requires this and this is a
   // member function, set its alignment accordingly.
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));
   }
 


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2401,7 +2401,7 @@
   // functions. If the current target's C++ ABI requires this and this is a
   // member function, set its alignment accordingly.
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2389
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));

daltenty wrote:
> v.g.vassilev wrote:
> > daltenty wrote:
> > > Thanks for looking into this. 
> > > 
> > > It's not clear to me how this re-ordering ends up fixing things. Can you 
> > > clarify what the uninitialized value was in this expression?
> > > 
> > > 
> > The issue happens only in Release builds (RelWithDebInfo, too). From what I 
> > was able to see it is somewhere in `F->getPointerAlignment`. My assumption 
> > was that we cannot rely on the full properties of `F` to be set unless it 
> > is the declaration kind we expected (similar to checking if a something is 
> > a nullptr and then probing its members). Secondly the `isa` check is likely 
> > to be the less expensive check anyway.
> > 
> > I saw the issue yesterday and dug into it for a while. However, I decided 
> > to insert a "fix" before the release which is in few days since the `isa` 
> > seems to the faster check anyway.
> Thanks, yeah thats kind of what I expected.  `F->getPointerAlignment()` is 
> likely getting inlined into in to this callsite and we are inspecting 
> uninitialized properties of the DataLayout. The weird part is I don't see why 
> those properties of the DataLayout ever should be uninitialized, so I think 
> there might be something more broken underneath this.
> 
> That said, this is definitely better than before as you say, so let's go 
> ahead with this for the release and maybe I'll do some more digging in 
> `getPointerAlignment`.
Sounds good. For better or worse that workaround might make the issue more 
subtle to debug next time it appears...


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2389
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));

daltenty wrote:
> Thanks for looking into this. 
> 
> It's not clear to me how this re-ordering ends up fixing things. Can you 
> clarify what the uninitialized value was in this expression?
> 
> 
The issue happens only in Release builds (RelWithDebInfo, too). From what I was 
able to see it is somewhere in `F->getPointerAlignment`. My assumption was that 
we cannot rely on the full properties of `F` to be set unless it is the 
declaration kind we expected (similar to checking if a something is a nullptr 
and then probing its members). Secondly the `isa` check is likely to be the 
less expensive check anyway.

I saw the issue yesterday and dug into it for a while. However, I decided to 
insert a "fix" before the release which is in few days since the `isa` seems to 
the faster check anyway.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.
v.g.vassilev added reviewers: rjmccall, daltenty, aaron.ballman.
Herald added a subscriber: pengfei.
Herald added a project: All.
v.g.vassilev requested review of this revision.
Herald added a subscriber: wangpc.

This patch fixes valgrind reports from downstream consumers about conditional 
jump over uninitialised.

  [ RUN  ] ScopeReflectionTest.IsComplete
  ==987150== Conditional jump or move depends on uninitialised value(s)
  ==987150==at 0x1E1128F: 
clang::CodeGen::CodeGenModule::SetLLVMFunctionAttributesForDefinition(clang::Decl
 const*, llvm::Function*) (CodeGenModule.cpp:2391)
  ==987150==by 0x1E4F181: 
clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition(clang::GlobalDecl, 
llvm::GlobalValue*) (CodeGenModule.cpp:5669)
  ==987150==by 0x1E4A194: 
clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl, 
llvm::GlobalValue*) (CodeGenModule.cpp:3909)
  ==987150==by 0x1E4A752: 
clang::CodeGen::CodeGenModule::EmitGlobal(clang::GlobalDecl) 
(CodeGenModule.cpp:3649)
  ==987150==by 0x1E532F5: 
clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) [clone .part.0] 
(CodeGenModule.cpp:6563)
  ==987150==by 0x1B0BEDD: (anonymous 
namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) 
(ModuleBuilder.cpp:190)
  ==987150==by 0x1AEA47B: 
clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) 
(CodeGenAction.cpp:235)
  ==987150==by 0x101B02F: 
clang::IncrementalASTConsumer::HandleTopLevelDecl(clang::DeclGroupRef) 
(IncrementalParser.cpp:52)
  ==987150==by 0x101ED93: 
clang::IncrementalParser::ParseOrWrapTopLevelDecl() (IncrementalParser.cpp:276)
  ==987150==by 0x101FBBC: clang::IncrementalParser::Parse(llvm::StringRef) 
(IncrementalParser.cpp:342)
  ==987150==by 0x100E104: clang::Interpreter::Parse(llvm::StringRef) 
(Interpreter.cpp:360)
  ==987150==by 0xE734C0: Cpp::Interpreter::Parse(llvm::StringRef) 
(CppInterOpInterpreter.h:172)
  ==987150==  Uninitialised value was created by a heap allocation
  ==987150==at 0x844BE63: operator new(unsigned long) (in 
/usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==987150==by 0x1B0C882: StartModule (ModuleBuilder.cpp:139)
  ==987150==by 0x1B0C882: 
clang::CodeGenerator::StartModule(llvm::StringRef, llvm::LLVMContext&) 
(ModuleBuilder.cpp:360)
  ==987150==by 0x101C4AF: clang::IncrementalParser::GenModule() 
(IncrementalParser.cpp:372)
  ==987150==by 0x101FC0E: clang::IncrementalParser::Parse(llvm::StringRef) 
(IncrementalParser.cpp:362)
  ==987150==by 0x100E104: clang::Interpreter::Parse(llvm::StringRef) 
(Interpreter.cpp:360)
  ==987150==by 0x100E243: 
clang::Interpreter::create(std::unique_ptr >) (Interpreter.cpp:279)
  ==987150==by 0xF2131A: compat::createClangInterpreter(std::vector >&) (Compatibility.h:123)
  ==987150==by 0xF22AB9: Cpp::Interpreter::Interpreter(int, char const* 
const*, char const*, std::vector, 
std::allocator > > const&, void*, 
bool) (CppInterOpInterpreter.h:146)
  ==987150==by 0xF1827A: CreateInterpreter (CppInterOp.cpp:2494)
  ==987150==by 0xECFA0E: 
TestUtils::GetAllTopLevelDecls(std::__cxx11::basic_string, std::allocator > const&, 
std::vector >&, bool) (Utils.cpp:23)
  ==987150==by 0xE9CB85: ScopeReflectionTest_IsComplete_Test::TestBody() 
(ScopeReflectionTest.cpp:71)
  ==987150==by 0xF0ED0C: void 
testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in 
/home/vvassilev/workspace/builds/scratch/cppyy/InterOp/build-with-clang-repl-release/unittests/CppInterOp/CppInterOpTests)
  ==987150== 


Repository:
  rC Clang

https://reviews.llvm.org/D159339

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2386,7 +2386,7 @@
   // functions. If the current target's C++ ABI requires this and this is a
   // member function, set its alignment accordingly.
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));
   }
 


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2386,7 +2386,7 @@
   // functions. If the current target's C++ ABI requires this and this is a
   // member function, set its alignment accordingly.
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), 

[PATCH] D159115: [clang-repl] Adapt to the recent dylib-related changes in ORC.

[Vassil Vassilev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG196d8569d46d: [clang-repl] Adapt to the recent dylib-related 
changes in ORC. (authored by v.g.vassilev).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159115/new/

https://reviews.llvm.org/D159115

Files:
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp


Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -234,10 +234,16 @@
   }
 
   std::string MangledName = MangleName(FD);
-  auto Addr = cantFail(Interp->getSymbolAddress(MangledName));
-  EXPECT_NE(0U, Addr.getValue());
+  auto Addr = Interp->getSymbolAddress(MangledName);
+  EXPECT_FALSE(!Addr);
+  EXPECT_NE(0U, Addr->getValue());
   GlobalDecl GD(FD);
-  EXPECT_EQ(Addr, cantFail(Interp->getSymbolAddress(GD)));
+  EXPECT_EQ(*Addr, cantFail(Interp->getSymbolAddress(GD)));
+  cantFail(
+  Interp->ParseAndExecute("extern \"C\" int printf(const char*,...);"));
+  Addr = Interp->getSymbolAddress("printf");
+  EXPECT_FALSE(!Addr);
+  EXPECT_EQ((unsigned long long), Addr->getValue());
 }
 
 static void *AllocateObject(TypeDecl *TD, Interpreter ) {
Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -92,12 +92,19 @@
 llvm::Expected
 IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
   SymbolNameKind NameKind) const {
-  auto Sym = (NameKind == LinkerName) ? Jit->lookupLinkerMangled(Name)
-  : Jit->lookup(Name);
-
-  if (!Sym)
-return Sym.takeError();
-  return Sym;
+  using namespace llvm::orc;
+  auto SO = makeJITDylibSearchOrder({>getMainJITDylib(),
+ Jit->getPlatformJITDylib().get(),
+ Jit->getProcessSymbolsJITDylib().get()});
+
+  ExecutionSession  = Jit->getExecutionSession();
+
+  auto SymOrErr =
+  ES.lookup(SO, (NameKind == LinkerName) ? ES.intern(Name)
+ : Jit->mangleAndIntern(Name));
+  if (auto Err = SymOrErr.takeError())
+return std::move(Err);
+  return SymOrErr->getAddress();
 }
 
 } // end namespace clang


Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -234,10 +234,16 @@
   }
 
   std::string MangledName = MangleName(FD);
-  auto Addr = cantFail(Interp->getSymbolAddress(MangledName));
-  EXPECT_NE(0U, Addr.getValue());
+  auto Addr = Interp->getSymbolAddress(MangledName);
+  EXPECT_FALSE(!Addr);
+  EXPECT_NE(0U, Addr->getValue());
   GlobalDecl GD(FD);
-  EXPECT_EQ(Addr, cantFail(Interp->getSymbolAddress(GD)));
+  EXPECT_EQ(*Addr, cantFail(Interp->getSymbolAddress(GD)));
+  cantFail(
+  Interp->ParseAndExecute("extern \"C\" int printf(const char*,...);"));
+  Addr = Interp->getSymbolAddress("printf");
+  EXPECT_FALSE(!Addr);
+  EXPECT_EQ((unsigned long long), Addr->getValue());
 }
 
 static void *AllocateObject(TypeDecl *TD, Interpreter ) {
Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -92,12 +92,19 @@
 llvm::Expected
 IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
   SymbolNameKind NameKind) const {
-  auto Sym = (NameKind == LinkerName) ? Jit->lookupLinkerMangled(Name)
-  : Jit->lookup(Name);
-
-  if (!Sym)
-return Sym.takeError();
-  return Sym;
+  using namespace llvm::orc;
+  auto SO = makeJITDylibSearchOrder({>getMainJITDylib(),
+ Jit->getPlatformJITDylib().get(),
+ Jit->getProcessSymbolsJITDylib().get()});
+
+  ExecutionSession  = Jit->getExecutionSession();
+
+  auto SymOrErr =
+  ES.lookup(SO, (NameKind == LinkerName) ? ES.intern(Name)
+ : Jit->mangleAndIntern(Name));
+  if (auto Err = SymOrErr.takeError())
+return std::move(Err);
+  return SymOrErr->getAddress();
 }
 
 } // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159115: [clang-repl] Adapt to the recent dylib-related changes in ORC.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 554385.
v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.

Address style comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159115/new/

https://reviews.llvm.org/D159115

Files:
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp


Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -234,10 +234,16 @@
   }
 
   std::string MangledName = MangleName(FD);
-  auto Addr = cantFail(Interp->getSymbolAddress(MangledName));
-  EXPECT_NE(0U, Addr.getValue());
+  auto Addr = Interp->getSymbolAddress(MangledName);
+  EXPECT_FALSE(!Addr);
+  EXPECT_NE(0U, Addr->getValue());
   GlobalDecl GD(FD);
-  EXPECT_EQ(Addr, cantFail(Interp->getSymbolAddress(GD)));
+  EXPECT_EQ(*Addr, cantFail(Interp->getSymbolAddress(GD)));
+  cantFail(
+  Interp->ParseAndExecute("extern \"C\" int printf(const char*,...);"));
+  Addr = Interp->getSymbolAddress("printf");
+  EXPECT_FALSE(!Addr);
+  EXPECT_EQ((unsigned long long), Addr->getValue());
 }
 
 static void *AllocateObject(TypeDecl *TD, Interpreter ) {
Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -92,12 +92,19 @@
 llvm::Expected
 IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
   SymbolNameKind NameKind) const {
-  auto Sym = (NameKind == LinkerName) ? Jit->lookupLinkerMangled(Name)
-  : Jit->lookup(Name);
-
-  if (!Sym)
-return Sym.takeError();
-  return Sym;
+  using namespace llvm::orc;
+  auto SO = makeJITDylibSearchOrder({>getMainJITDylib(),
+ Jit->getPlatformJITDylib().get(),
+ Jit->getProcessSymbolsJITDylib().get()});
+
+  ExecutionSession  = Jit->getExecutionSession();
+
+  auto SymOrErr =
+  ES.lookup(SO, (NameKind == LinkerName) ? ES.intern(Name)
+ : Jit->mangleAndIntern(Name));
+  if (auto Err = SymOrErr.takeError())
+return std::move(Err);
+  return SymOrErr->getAddress();
 }
 
 } // end namespace clang


Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -234,10 +234,16 @@
   }
 
   std::string MangledName = MangleName(FD);
-  auto Addr = cantFail(Interp->getSymbolAddress(MangledName));
-  EXPECT_NE(0U, Addr.getValue());
+  auto Addr = Interp->getSymbolAddress(MangledName);
+  EXPECT_FALSE(!Addr);
+  EXPECT_NE(0U, Addr->getValue());
   GlobalDecl GD(FD);
-  EXPECT_EQ(Addr, cantFail(Interp->getSymbolAddress(GD)));
+  EXPECT_EQ(*Addr, cantFail(Interp->getSymbolAddress(GD)));
+  cantFail(
+  Interp->ParseAndExecute("extern \"C\" int printf(const char*,...);"));
+  Addr = Interp->getSymbolAddress("printf");
+  EXPECT_FALSE(!Addr);
+  EXPECT_EQ((unsigned long long), Addr->getValue());
 }
 
 static void *AllocateObject(TypeDecl *TD, Interpreter ) {
Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -92,12 +92,19 @@
 llvm::Expected
 IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
   SymbolNameKind NameKind) const {
-  auto Sym = (NameKind == LinkerName) ? Jit->lookupLinkerMangled(Name)
-  : Jit->lookup(Name);
-
-  if (!Sym)
-return Sym.takeError();
-  return Sym;
+  using namespace llvm::orc;
+  auto SO = makeJITDylibSearchOrder({>getMainJITDylib(),
+ Jit->getPlatformJITDylib().get(),
+ Jit->getProcessSymbolsJITDylib().get()});
+
+  ExecutionSession  = Jit->getExecutionSession();
+
+  auto SymOrErr =
+  ES.lookup(SO, (NameKind == LinkerName) ? ES.intern(Name)
+ : Jit->mangleAndIntern(Name));
+  if (auto Err = SymOrErr.takeError())
+return std::move(Err);
+  return SymOrErr->getAddress();
 }
 
 } // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159115: [clang-repl] Adapt to the recent dylib-related changes in ORC.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 554378.
v.g.vassilev added a comment.

Address review comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159115/new/

https://reviews.llvm.org/D159115

Files:
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp


Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -234,10 +234,16 @@
   }
 
   std::string MangledName = MangleName(FD);
-  auto Addr = cantFail(Interp->getSymbolAddress(MangledName));
-  EXPECT_NE(0U, Addr.getValue());
+  auto Addr = Interp->getSymbolAddress(MangledName);
+  EXPECT_FALSE(!Addr);
+  EXPECT_NE(0U, Addr->getValue());
   GlobalDecl GD(FD);
-  EXPECT_EQ(Addr, cantFail(Interp->getSymbolAddress(GD)));
+  EXPECT_EQ(*Addr, cantFail(Interp->getSymbolAddress(GD)));
+  cantFail(
+  Interp->ParseAndExecute("extern \"C\" int printf(const char*,...);"));
+  Addr = Interp->getSymbolAddress("printf");
+  EXPECT_FALSE(!Addr);
+  EXPECT_EQ((unsigned long long), Addr->getValue());
 }
 
 static void *AllocateObject(TypeDecl *TD, Interpreter ) {
Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -92,12 +92,21 @@
 llvm::Expected
 IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
   SymbolNameKind NameKind) const {
-  auto Sym = (NameKind == LinkerName) ? Jit->lookupLinkerMangled(Name)
-  : Jit->lookup(Name);
-
-  if (!Sym)
-return Sym.takeError();
-  return Sym;
+  using namespace llvm::orc;
+  JITDylibSearchOrder O;
+  JITDylibLookupFlags Flags = JITDylibLookupFlags::MatchExportedSymbolsOnly;
+  O.push_back({>getMainJITDylib(), Flags});
+  O.push_back({Jit->getPlatformJITDylib().get(), Flags});
+  O.push_back({Jit->getProcessSymbolsJITDylib().get(), Flags});
+
+  ExecutionSession  = Jit->getExecutionSession();
+
+  auto SymOrErr =
+  ES.lookup(O, (NameKind == LinkerName) ? ES.intern(Name)
+: Jit->mangleAndIntern(Name));
+  if (auto Err = SymOrErr.takeError())
+return std::move(Err);
+  return SymOrErr->getAddress();
 }
 
 } // end namespace clang


Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -234,10 +234,16 @@
   }
 
   std::string MangledName = MangleName(FD);
-  auto Addr = cantFail(Interp->getSymbolAddress(MangledName));
-  EXPECT_NE(0U, Addr.getValue());
+  auto Addr = Interp->getSymbolAddress(MangledName);
+  EXPECT_FALSE(!Addr);
+  EXPECT_NE(0U, Addr->getValue());
   GlobalDecl GD(FD);
-  EXPECT_EQ(Addr, cantFail(Interp->getSymbolAddress(GD)));
+  EXPECT_EQ(*Addr, cantFail(Interp->getSymbolAddress(GD)));
+  cantFail(
+  Interp->ParseAndExecute("extern \"C\" int printf(const char*,...);"));
+  Addr = Interp->getSymbolAddress("printf");
+  EXPECT_FALSE(!Addr);
+  EXPECT_EQ((unsigned long long), Addr->getValue());
 }
 
 static void *AllocateObject(TypeDecl *TD, Interpreter ) {
Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -92,12 +92,21 @@
 llvm::Expected
 IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
   SymbolNameKind NameKind) const {
-  auto Sym = (NameKind == LinkerName) ? Jit->lookupLinkerMangled(Name)
-  : Jit->lookup(Name);
-
-  if (!Sym)
-return Sym.takeError();
-  return Sym;
+  using namespace llvm::orc;
+  JITDylibSearchOrder O;
+  JITDylibLookupFlags Flags = JITDylibLookupFlags::MatchExportedSymbolsOnly;
+  O.push_back({>getMainJITDylib(), Flags});
+  O.push_back({Jit->getPlatformJITDylib().get(), Flags});
+  O.push_back({Jit->getProcessSymbolsJITDylib().get(), Flags});
+
+  ExecutionSession  = Jit->getExecutionSession();
+
+  auto SymOrErr =
+  ES.lookup(O, (NameKind == LinkerName) ? ES.intern(Name)
+: Jit->mangleAndIntern(Name));
+  if (auto Err = SymOrErr.takeError())
+return std::move(Err);
+  return SymOrErr->getAddress();
 }
 
 } // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159115: [clang-repl] Adapt to the recent dylib-related changes in ORC.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.
v.g.vassilev added reviewers: lhames, sunho.
Herald added a project: All.
v.g.vassilev requested review of this revision.

ORC splits into separate dylibs symbols coming from the process and symbols 
materialized in the Jit. This patch adapts intent of the existing interface and 
adds a regression test to make sure both Jit'd and compiled symbols can be 
found.


Repository:
  rC Clang

https://reviews.llvm.org/D159115

Files:
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp


Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -234,10 +234,16 @@
   }
 
   std::string MangledName = MangleName(FD);
-  auto Addr = cantFail(Interp->getSymbolAddress(MangledName));
-  EXPECT_NE(0U, Addr.getValue());
+  auto Addr = Interp->getSymbolAddress(MangledName);
+  EXPECT_FALSE(!Addr);
+  EXPECT_NE(0U, Addr->getValue());
   GlobalDecl GD(FD);
-  EXPECT_EQ(Addr, cantFail(Interp->getSymbolAddress(GD)));
+  EXPECT_EQ(*Addr, cantFail(Interp->getSymbolAddress(GD)));
+  cantFail(
+  Interp->ParseAndExecute("extern \"C\" int printf(const char*,...);"));
+  Addr = Interp->getSymbolAddress("printf");
+  EXPECT_FALSE(!Addr);
+  EXPECT_EQ((unsigned long long), Addr->getValue());
 }
 
 static void *AllocateObject(TypeDecl *TD, Interpreter ) {
Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -92,12 +92,20 @@
 llvm::Expected
 IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
   SymbolNameKind NameKind) const {
-  auto Sym = (NameKind == LinkerName) ? Jit->lookupLinkerMangled(Name)
-  : Jit->lookup(Name);
-
-  if (!Sym)
-return Sym.takeError();
-  return Sym;
+  using namespace llvm::orc;
+  JITDylibSearchOrder O;
+  JITDylibLookupFlags Flags = JITDylibLookupFlags::MatchExportedSymbolsOnly;
+  O.push_back({>getMainJITDylib(), Flags});
+  O.push_back({Jit->getPlatformJITDylib().get(), Flags});
+  O.push_back({Jit->getProcessSymbolsJITDylib().get(), Flags});
+
+  ExecutionSession  = Jit->getExecutionSession();
+
+  auto SymOrErr = ES.lookup(
+  O, ES.intern((NameKind == LinkerName) ? Name : Jit->mangle(Name)));
+  if (auto Err = SymOrErr.takeError())
+return std::move(Err);
+  return SymOrErr->getAddress();
 }
 
 } // end namespace clang


Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -234,10 +234,16 @@
   }
 
   std::string MangledName = MangleName(FD);
-  auto Addr = cantFail(Interp->getSymbolAddress(MangledName));
-  EXPECT_NE(0U, Addr.getValue());
+  auto Addr = Interp->getSymbolAddress(MangledName);
+  EXPECT_FALSE(!Addr);
+  EXPECT_NE(0U, Addr->getValue());
   GlobalDecl GD(FD);
-  EXPECT_EQ(Addr, cantFail(Interp->getSymbolAddress(GD)));
+  EXPECT_EQ(*Addr, cantFail(Interp->getSymbolAddress(GD)));
+  cantFail(
+  Interp->ParseAndExecute("extern \"C\" int printf(const char*,...);"));
+  Addr = Interp->getSymbolAddress("printf");
+  EXPECT_FALSE(!Addr);
+  EXPECT_EQ((unsigned long long), Addr->getValue());
 }
 
 static void *AllocateObject(TypeDecl *TD, Interpreter ) {
Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -92,12 +92,20 @@
 llvm::Expected
 IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
   SymbolNameKind NameKind) const {
-  auto Sym = (NameKind == LinkerName) ? Jit->lookupLinkerMangled(Name)
-  : Jit->lookup(Name);
-
-  if (!Sym)
-return Sym.takeError();
-  return Sym;
+  using namespace llvm::orc;
+  JITDylibSearchOrder O;
+  JITDylibLookupFlags Flags = JITDylibLookupFlags::MatchExportedSymbolsOnly;
+  O.push_back({>getMainJITDylib(), Flags});
+  O.push_back({Jit->getPlatformJITDylib().get(), Flags});
+  O.push_back({Jit->getProcessSymbolsJITDylib().get(), Flags});
+
+  ExecutionSession  = Jit->getExecutionSession();
+
+  auto SymOrErr = ES.lookup(
+  O, ES.intern((NameKind == LinkerName) ? Name : Jit->mangle(Name)));
+  if (auto Err = SymOrErr.takeError())
+return std::move(Err);
+  return SymOrErr->getAddress();
 }
 
 } // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159018: [clang][modules] Add a c23 module feature

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM!

In D159018#4622598 , @iana wrote:

> Would we want to back port this to llvm 17?

Yes probably.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159018/new/

https://reviews.llvm.org/D159018

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ab25a42ba70: Reland [clang-repl] support code 
completion at a REPL. (authored by capfredf, committed by v.g.vassilev).

Changed prior to commit:
  https://reviews.llvm.org/D154382?vs=554002=554027#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang/include/clang/Frontend/ASTUnit.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/tools/clang-repl/ClangRepl.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  clang/unittests/Interpreter/CMakeLists.txt

Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -9,6 +9,7 @@
 add_clang_unittest(ClangReplInterpreterTests
   IncrementalProcessingTest.cpp
   InterpreterTest.cpp
+  CodeCompletionTest.cpp
   )
 target_link_libraries(ClangReplInterpreterTests PUBLIC
   clangAST
Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -11,8 +11,8 @@
 //
 //===--===//
 
-#include "CIndexer.h"
 #include "CIndexDiagnostic.h"
+#include "CIndexer.h"
 #include "CLog.h"
 #include "CXCursor.h"
 #include "CXSourceLocation.h"
@@ -25,6 +25,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/SmallString.h"
@@ -41,7 +42,6 @@
 #include 
 #include 
 
-
 #ifdef UDP_CODE_COMPLETION_LOGGER
 #include "clang/Basic/Version.h"
 #include 
@@ -543,6 +543,7 @@
 case CodeCompletionContext::CCC_PreprocessorExpression:
 case CodeCompletionContext::CCC_PreprocessorDirective:
 case CodeCompletionContext::CCC_Attribute:
+case CodeCompletionContext::CCC_TopLevelOrExpression:
 case CodeCompletionContext::CCC_TypeQualifiers: {
   //Only Clang results should be accepted, so we'll set all of the other
   //context bits to 0 (i.e. the empty set)
Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -13,6 +13,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
+#include "clang/Interpreter/CodeCompletion.h"
 #include "clang/Interpreter/Interpreter.h"
 
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
@@ -70,6 +71,70 @@
   return (Errs || HasError) ? EXIT_FAILURE : EXIT_SUCCESS;
 }
 
+struct ReplListCompleter {
+  clang::IncrementalCompilerBuilder 
+  clang::Interpreter 
+  ReplListCompleter(clang::IncrementalCompilerBuilder ,
+clang::Interpreter )
+  : CB(CB), MainInterp(Interp){};
+
+  std::vector operator()(llvm::StringRef Buffer,
+   size_t Pos) const;
+  std::vector
+  operator()(llvm::StringRef Buffer, size_t Pos, llvm::Error ) const;
+};
+
+std::vector
+ReplListCompleter::operator()(llvm::StringRef Buffer, size_t Pos) const {
+  auto Err = llvm::Error::success();
+  auto res = (*this)(Buffer, Pos, Err);
+  if (Err)
+llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
+  return res;
+}
+
+std::vector
+ReplListCompleter::operator()(llvm::StringRef Buffer, size_t Pos,
+  llvm::Error ) const {
+  std::vector Comps;
+  std::vector Results;
+
+  auto CI = CB.CreateCpp();
+  if (auto Err = CI.takeError()) {
+ErrRes = std::move(Err);
+return {};
+  }
+
+  size_t Lines =
+  std::count(Buffer.begin(), std::next(Buffer.begin(), Pos), '\n') + 1;
+  auto Interp = clang::Interpreter::create(std::move(*CI));
+
+  if (auto Err = Interp.takeError()) {
+// log the error and returns an empty vector;
+ErrRes = std::move(Err);
+
+return {};
+  }
+
+  codeComplete(
+  const_cast((*Interp)->getCompilerInstance()),
+  Buffer, Lines, Pos + 1, MainInterp.getCompilerInstance(), Results);
+
+  size_t space_pos = Buffer.rfind(" ");
+  llvm::StringRef 

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/unittests/Interpreter/CodeCompletionTest.cpp:38
+
+  std::vector Results;
+

We should make this an out parameter instead of returning it by copy.



Comment at: clang/unittests/Interpreter/CodeCompletionTest.cpp:42
+  const_cast((*Interp)->getCompilerInstance()),
+  Prefix, 1, Prefix.size(), MainInterp.getCompilerInstance(), Results);
+




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Interpreter/WASM.cpp:79
+  int Result =
+  lld::wasm::link(LinkerArgs, llvm::outs(), llvm::errs(), false, false);
+  if (!Result)

sbc100 wrote:
> v.g.vassilev wrote:
> > sbc100 wrote:
> > > argentite wrote:
> > > > v.g.vassilev wrote:
> > > > > I am not sure how we can solve that dependency here. Worst case 
> > > > > scenario, could we check if `lld` is installed and make a system call?
> > > > AFAIK we can't really `exec()` within Emscripten.
> > > This looks its using the in-process call to the linker library, rather 
> > > then an exec of the linker process.. which could work.   But is it OK to 
> > > have the compiler depend on the linker like this?
> > Well, this is what we would like to avoid. Somehow. Initially, I thought we 
> > could move the relevant lld bits in the ORC infrastructure but maybe that’s 
> > not a good idea after all. Is there a suitable place where we could move 
> > that lld logic in LLVM?
> Perhaps you could wrap all of this in `if __EMSCRIPTEN__` or even something 
> like `if STATICALLY_LINKED_LLD` so it would only be available in your special 
> build of the compiler?
@argentite could we try @sbc100's proposal?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158140/new/

https://reviews.llvm.org/D158140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151904: [clang-repl][CUDA] Add an unit test for interactive CUDA

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

@argentite ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151904/new/

https://reviews.llvm.org/D151904

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Interpreter/WASM.cpp:79
+  int Result =
+  lld::wasm::link(LinkerArgs, llvm::outs(), llvm::errs(), false, false);
+  if (!Result)

sbc100 wrote:
> argentite wrote:
> > v.g.vassilev wrote:
> > > I am not sure how we can solve that dependency here. Worst case scenario, 
> > > could we check if `lld` is installed and make a system call?
> > AFAIK we can't really `exec()` within Emscripten.
> This looks its using the in-process call to the linker library, rather then 
> an exec of the linker process.. which could work.   But is it OK to have the 
> compiler depend on the linker like this?
Well, this is what we would like to avoid. Somehow. Initially, I thought we 
could move the relevant lld bits in the ORC infrastructure but maybe that’s not 
a good idea after all. Is there a suitable place where we could move that lld 
logic in LLVM?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158140/new/

https://reviews.llvm.org/D158140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[Vassil Vassilev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3edd338a6407: [clang-repl] Add Documentation for Execution 
Results Handling. (authored by Krishna-13-cyber, committed by v.g.vassilev).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

Files:
  clang/docs/ClangRepl.rst
  clang/docs/conf.py

Index: clang/docs/conf.py
===
--- clang/docs/conf.py
+++ clang/docs/conf.py
@@ -49,6 +49,7 @@
 if sphinx.version_info >= (3, 0):
 # This requires 0.5 or later.
 extensions.append("recommonmark")
+extensions.append('sphinx.ext.graphviz')
 else:
 source_parsers = {".md": "recommonmark.parser.CommonMarkParser"}
 source_suffix[".md"] = "markdown"
Index: clang/docs/ClangRepl.rst
===
--- clang/docs/ClangRepl.rst
+++ clang/docs/ClangRepl.rst
@@ -213,6 +213,411 @@
 automatic language interoperability. It also helps static languages such as C/C++ become
 apt for data science.
 
+Execution Results Handling in Clang-Repl
+
+
+Execution Results Handling features discussed below help extend the Clang-Repl
+functionality by creating an interface between the execution results of a
+program and the compiled program.
+
+1. **Capture Execution Results**: This feature helps capture the execution results
+of a program and bring them back to the compiled program.
+
+2. **Dump Captured Execution Results**: This feature helps create a temporary dump
+for Value Printing/Automatic Printf, that is, to display the value and type of
+the captured data.
+
+
+1. Capture Execution Results
+
+
+In many cases, it is useful to bring back the program execution result to the
+compiled program. This result can be stored in an object of type **Value**.
+
+How Execution Results are captured (Value Synthesis):
+-
+
+The synthesizer chooses which expression to synthesize, and then it replaces
+the original expression with the synthesized expression. Depending on the
+expression type, it may choose to save an object (``LastValue``) of type 'value'
+while allocating memory to it (``SetValueWithAlloc()``), or not (
+``SetValueNoAlloc()``).
+
+.. graphviz::
+:name: valuesynthesis
+:caption: Value Synthesis
+:alt: Shows how an object of type 'Value' is synthesized
+:align: center
+
+ digraph "valuesynthesis" {
+ rankdir="LR";
+ graph [fontname="Verdana", fontsize="12"];
+ node [fontname="Verdana", fontsize="12"];
+ edge [fontname="Sans", fontsize="9"];
+
+ start [label=" Create an Object \n 'Last Value' \n of type 'Value' ", shape="note", fontcolor=white, fillcolor="#ff", style=filled];
+ assign [label=" Assign the result \n to the 'LastValue' \n (based on respective \n Memory Allocation \n scenario) ", shape="box"]
+ print [label=" Pretty Print \n the Value Object ", shape="Msquare", fillcolor="yellow", style=filled];
+ start -> assign;
+ assign -> print;
+
+   subgraph SynthesizeExpression {
+ synth [label=" SynthesizeExpr() ", shape="note", fontcolor=white, fillcolor="#ff", style=filled];
+ mem [label=" New Memory \n Allocation? ", shape="diamond"];
+ withaloc [label=" SetValueWithAlloc() ", shape="box"];
+ noaloc [label=" SetValueNoAlloc() ", shape="box"];
+ right [label=" 1. RValue Structure \n (a temporary value)", shape="box"];
+ left2 [label=" 2. LValue Structure \n (a variable with \n an address)", shape="box"];
+ left3 [label=" 3. Built-In Type \n (int, float, etc.)", shape="box"];
+ output [label=" move to 'Assign' step ", shape="box"];
+
+ synth -> mem;
+ mem -> withaloc [label="Yes"];
+ mem -> noaloc [label="No"];
+ withaloc -> right;
+ noaloc -> left2;
+ noaloc -> left3;
+ right -> output;
+ left2 -> output;
+ left3 -> output;
+  }
+output -> assign
+  }
+
+Where is the captured result stored?
+
+
+``LastValue`` holds the last result of the value printing. It is a class member
+because it can be accessed even after subsequent inputs.
+
+**Note:** If no value printing happens, then it is in an invalid state.
+
+Improving Efficiency and User Experience
+
+
+The Value object is essentially used to create a mapping between an expression
+'type' and the allocated 'memory'. Built-in types (bool, char, int,
+float, double, etc.) are copyable. Their their memory allocation size is 

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.

The pre-merge check seems to fail at clang-format for some reason. This patch 
does not change anything that should be formatted by clang-format.

We will try to add the images as editable graphviz content. We need to see if 
the infrastructure allows for this as suggested here: 
https://discourse.llvm.org/t/any-tool-for-creating-editable-diagrams-in-llvm-clang-repl-documentation-similar-to-mermaid-ingithub/72729/2


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148997: [clang] Add a new annotation token: annot_repl_input_end

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D148997#4607833 , @bnbarham wrote:

> In D148997#4561620 , @v.g.vassilev 
> wrote:
>
>> So, in that case we should bring back the boolean flag for incremental 
>> processing and keep the `IncrementalExtensions` LanguageOption separate. In 
>> that case `IncrementalExtensions` would mean that we must turn on 
>> incremental processing for managing lifetime and only use the language 
>> option when extending the parsing logic. However, I think the problem would 
>> be what to do with the `tok::eof` and `tok::annot_repl_input_end`? I'd 
>> probably need @aaron.ballman or @rsmith here...
>
> Would you be happy to make that change, or should I put it up? Separating the 
> options and what to do about the token in general could be separate PRs.

If you could do it would be better as it would capture better your concrete use 
case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148997/new/

https://reviews.llvm.org/D148997

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeb0e6c3134ef: [clang-repl] support code completion at a 
REPL. (authored by capfredf, committed by v.g.vassilev).

Changed prior to commit:
  https://reviews.llvm.org/D154382?vs=552418=552696#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

Files:
  clang/include/clang/Frontend/ASTUnit.h
  clang/include/clang/Interpreter/CodeCompletion.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/CodeCompletion.cpp
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/incrememal-mode-completion-no-error.cpp
  clang/test/CodeCompletion/incremental-top-level.cpp
  clang/tools/clang-repl/ClangRepl.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/CodeCompletionTest.cpp

Index: clang/unittests/Interpreter/CodeCompletionTest.cpp
===
--- /dev/null
+++ clang/unittests/Interpreter/CodeCompletionTest.cpp
@@ -0,0 +1,101 @@
+#include "clang/Interpreter/CodeCompletion.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Interpreter/Interpreter.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/LineEditor/LineEditor.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+namespace {
+auto CB = clang::IncrementalCompilerBuilder();
+
+static std::unique_ptr createInterpreter() {
+  auto CI = cantFail(CB.CreateCpp());
+  return cantFail(clang::Interpreter::create(std::move(CI)));
+}
+
+static std::vector runComp(clang::Interpreter ,
+llvm::StringRef Prefix,
+llvm::Error ) {
+  auto CI = CB.CreateCpp();
+  if (auto Err = CI.takeError()) {
+ErrR = std::move(Err);
+return {};
+  }
+
+  auto Interp = clang::Interpreter::create(std::move(*CI));
+  if (auto Err = Interp.takeError()) {
+// log the error and returns an empty vector;
+ErrR = std::move(Err);
+
+return {};
+  }
+
+  std::vector Results;
+
+  codeComplete(
+  const_cast((*Interp)->getCompilerInstance()),
+  Prefix, 1, Prefix.size(), MainInterp.getCompilerInstance(), Results);
+
+  std::vector Comps;
+  for (auto c : convertToCodeCompleteStrings(Results)) {
+if (c.find(Prefix) == 0)
+  Comps.push_back(c.substr(Prefix.size()));
+  }
+
+  return Comps;
+}
+
+TEST(CodeCompletionTest, Sanity) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int foo = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "f", Err);
+  EXPECT_EQ((size_t)2, comps.size()); // foo and float
+  EXPECT_EQ(comps[0], std::string("oo"));
+  EXPECT_EQ((bool)Err, false);
+}
+
+TEST(CodeCompletionTest, SanityNoneValid) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int foo = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "babanana", Err);
+  EXPECT_EQ((size_t)0, comps.size()); // foo and float
+  EXPECT_EQ((bool)Err, false);
+}
+
+TEST(CodeCompletionTest, TwoDecls) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int application = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  if (auto R = Interp->ParseAndExecute("int apple = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "app", Err);
+  EXPECT_EQ((size_t)2, comps.size());
+  EXPECT_EQ((bool)Err, false);
+}
+
+TEST(CodeCompletionTest, CompFunDeclsNoError) {
+  auto Interp = createInterpreter();
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "void app(", Err);
+  EXPECT_EQ((bool)Err, false);
+}
+
+} // anonymous namespace
Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -9,6 +9,7 @@
 add_clang_unittest(ClangReplInterpreterTests
   IncrementalProcessingTest.cpp
   InterpreterTest.cpp
+  CodeCompletionTest.cpp
   )
 target_link_libraries(ClangReplInterpreterTests PUBLIC
   clangAST
Index: clang/tools/libclang/CIndexCodeCompletion.cpp

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added reviewers: pmatos, tlively.
v.g.vassilev added a comment.

Let's add more reviewers to see if we can figure out how to move forward here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158140/new/

https://reviews.llvm.org/D158140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

This is quite exciting! Most of it looks already good enough to me. See inline 
comment.




Comment at: clang/lib/Interpreter/WASM.cpp:79
+  int Result =
+  lld::wasm::link(LinkerArgs, llvm::outs(), llvm::errs(), false, false);
+  if (!Result)

I am not sure how we can solve that dependency here. Worst case scenario, could 
we check if `lld` is installed and make a system call?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158140/new/

https://reviews.llvm.org/D158140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157838: [clang-repl] Disambiguate declarations with private typedefs

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

This change looks reasonable to me.




Comment at: clang/test/Interpreter/disambiguate-decl-stmt.cpp:74
+// expected-error@-1 2 {{'Inner' is a private member of 'PR13642'}}
+
 // Deduction guide

Hahnfeld wrote:
> v.g.vassilev wrote:
> > Could we move the diagnostic-producing cases in a separate file?
> We could, but I'm actually not a big fan because it moves the expected 
> failing test away from the related disambiguation tests above...
Okay, fair enough.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157838/new/

https://reviews.llvm.org/D157838

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.

Thanks for working on this @capfredf. Let me know if I should land that for you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157838: [clang-repl] Disambiguate declarations with private typedefs

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Parse/ParseTentative.cpp:94
+  // involve namespaces and friend declarations.
+  if (NextToken().is(tok::identifier))
+return true;

I am wondering what is the false positive rate of this change? That is, if we 
enable incremental parsing by default (in a local build) and then run all tests 
which do not produce diagnostics. 



Comment at: clang/test/Interpreter/disambiguate-decl-stmt.cpp:74
+// expected-error@-1 2 {{'Inner' is a private member of 'PR13642'}}
+
 // Deduction guide

Could we move the diagnostic-producing cases in a separate file?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157838/new/

https://reviews.llvm.org/D157838

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

@sammccall could you take another look? It seems quite ready to me to land.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D156537#4587874 , @aaron.ballman 
wrote:

> In D156537#4587832 , @v.g.vassilev 
> wrote:
>
>> @Hahnfeld, let's move forward and rely on a post-commit review here if 
>> necessary.
>
> Because one of the codegen code owners has been active on the thread with 
> concerns, I think it's better to wait a bit longer for an explicit sign-off 
> that those concerns are addressed (this code impacts more than clang-repl).

D157379  seems to resolve the concern but 
indeed we can wait a little longer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156537/new/

https://reviews.llvm.org/D156537

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

@Hahnfeld, let's move forward and rely on a post-commit review here if 
necessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156537/new/

https://reviews.llvm.org/D156537

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157379: [CodeGen] Restrict addEmittedDeferredDecl to incremental extensions

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

@Hahnfeld, let's move forward and rely on a post-commit review here if 
necessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157379/new/

https://reviews.llvm.org/D157379

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Interpreter/CodeCompletion.cpp:1
+//===-- CodeCompletion.cpp - Code Completion for ClangRepl ---===//
+//

I would propose to rename this file to `InterpreterCodeCompletion.cpp` and 
implement the `Interpreter::codeComplete` interface. Then we can move all of 
the content of `ExternalSource.{h,cpp}`, `CodeCompletion.h` and 
`ExternalSource.h` in it. That will increase the encapsulation of the code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151904: [clang-repl][CUDA] Add an unit test for interactive CUDA

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

@argentite what is the fate of this patch? Should we move forward with it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151904/new/

https://reviews.llvm.org/D151904

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a reviewer: sammccall.
v.g.vassilev added a subscriber: sammccall.
v.g.vassilev added a comment.

This looks mostly good to me. I noticed that @sammccall has done some work in 
that area in clangd and I was wondering if he has some thoughts about this 
patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Interpreter/IncrementalParser.h:91
+
+class IncrementalSyntaxOnlyAction : public SyntaxOnlyAction {
+  const CompilerInstance *ParentCI;

Can we not move this class definition locally to the CodeComplete interface?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/docs/ClangRepl.rst:221
+
+Execution Results Handling features discussed below help extend the Clang-REPL 
+functionality by creating an interface between the execution results of a 

We should probably spell consistently `Clang-Repl`.



Comment at: clang/docs/ClangRepl.rst:248
+
+.. image:: valuesynth.png
+   :align: center

Can we replace that with its graphviz version?



Comment at: clang/docs/ClangRepl.rst:264
+The Value object is essentially used to create a mapping between an expression 
+'type' and the 'memory' to be allocated. Built-in types (bool, char, int, 
+float, double, etc.) are simpler, since their memory allocation size is known. 





Comment at: clang/docs/ClangRepl.rst:265
+'type' and the 'memory' to be allocated. Built-in types (bool, char, int, 
+float, double, etc.) are simpler, since their memory allocation size is known. 
+In case of objects, a pointer can be saved, since the size of the object is 





Comment at: clang/docs/ClangRepl.rst:266-267
+float, double, etc.) are simpler, since their memory allocation size is known. 
+In case of objects, a pointer can be saved, since the size of the object is 
+not known.
+





Comment at: clang/docs/ClangRepl.rst:269-274
+For further improvement, the underlying Clang Type is also identified. For 
+example, ``X(char, Char_S)``, where ``Char_S`` is the Clang type. Clang types 
are 
+very efficient, which is important since these will be used in hotspots (high 
+utilization areas of the program). The ``Value.h`` header file has a very low 
+token count and was developed with strict constraints in mind, since it can 
+affect the performance of the interpreter.





Comment at: clang/docs/ClangRepl.rst:276-278
+This also enables the user to receive the computed 'type' back in their code 
+and then transform the type into something else (e.g., transform a double into 
+a float). Normally, the compiler can handle these conversions transparently, 





Comment at: clang/docs/ClangRepl.rst:284
+On-request conversions can help improve the user experience, by allowing 
+conversion to a desired 'to' type, when the 'from' type is unknown or unclear
+





Comment at: clang/docs/ClangRepl.rst:296-298
+For example, the CPPYY code makes use of this feature to enable running 
+C++ within Python. It enables transporting values/information between C++ 
+and Python.

QuillPusher wrote:
> @Krishna-13-cyber 
> 
> Please change CPPYY to cppyy
> 
> I just saw they use small caps on their official website




Comment at: clang/docs/ClangRepl.rst:352
+
+In Clang-REPL there is **interpreted code**, and this feature adds a 'value' 
+runtime that can talk to the **compiled code**.





Comment at: clang/docs/ClangRepl.rst:404
+this feature added to upstream Clang repo has essentially extended the syntax 
of
+C++,so that it can be more helpful for people that are writing code for data 
+science applications.





Comment at: clang/docs/ClangRepl.rst:419
+
+The Interpreter in Clang-REPL (``interpreter.cpp``) includes the function 
+``ParseAndExecute()`` that can accept a 'Value' parameter to capture the 
result. 





Comment at: clang/docs/ClangRepl.rst:461
+
+This feature uses a new token (annot_repl_input_end) to consider printing the 
+value of an expression if it doesn't end with a semicolon. When parsing an 




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156897: [CodeGen] Clean up access to EmittedDeferredDecls, NFCI.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

Looks reasonable to me. Let's give @rjmccall couple of more days to respond.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156897/new/

https://reviews.llvm.org/D156897

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157480: [clang-repl] Disambiguate global namespace identifiers

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157480/new/

https://reviews.llvm.org/D157480

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157477: [clang-repl] Add test for disambiguation of templates

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

If you have not merged this patch please consider my comment.




Comment at: clang/test/Interpreter/namespace-template-disambiguate.cpp:7
+namespace NS2 { struct A { public: using S = int; }; }
+namespace NS2 { A::S f(A::S a); }
+

It might be a good idea to add this test to `disambiguate-decl-stmt.cpp` where 
we track these things.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157477/new/

https://reviews.llvm.org/D157477

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157480: [clang-repl] Disambiguate global namespace identifiers

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/test/Interpreter/global-namespace-disambiguate.cpp:6
+struct A { ~A(); };
+::A::~A() {}
+

Can we add this test to `disambiguate-decl-stmt.cpp` instead where we track 
these things?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157480/new/

https://reviews.llvm.org/D157480

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/docs/ExecutionResultsHandling.rst:80
+
+For example, the CPPYY code makes use of this feature to enable running 
+C++ within Python. It enables transporting values/information between C++ 

cppyy seems undefined here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157477: [clang-repl] Add test for disambiguation of templates

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

Thanks, @Hahnfeld!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157477/new/

https://reviews.llvm.org/D157477

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157379: [CodeGen] Restrict addEmittedDeferredDecl to incremental extensions

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

This LGTM but I'd like to wait for @rjmccall for a week or so.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157379/new/

https://reviews.llvm.org/D157379

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148997: [clang] Add a new annotation token: annot_repl_input_end

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D148997#4561614 , @bnbarham wrote:

> In D148997#4561577 , @v.g.vassilev 
> wrote:
>
>>> My other concern here is that it seems our use cases are somewhat 
>>> different, eg. we wouldn't want any parsing differences and while I don't 
>>> know why this is yet, the removal of:
>>
>> I think I understand now. We basically want a mode which keeps some of the 
>> clang objects alive and ready to process more input. And on top, for 
>> clang-repl we want to enable some special parsing mode for the top-level 
>> statement declarations.
>
> Yeah, I'd say that sums it up pretty well.
>
>>>   // Skip over the EOF token, flagging end of previous input for incremental
>>>   // processing
>>>   if (PP.isIncrementalProcessingEnabled() && Tok.is(tok::eof))
>>> ConsumeToken();
>>>
>>> is causing issues for us as well.
>>
>> Yes, that now seems to be moved on the user side as we thought it would be 
>> dead code. We can bring that one back if that's the only issue you see with 
>> that approach.
>
> I think it would make sense to bring it back under the mode you spoke about 
> above, yeah 

So, in that case we should bring back the boolean flag for incremental 
processing and keep the `IncrementalExtensions` LanguageOption separate. In 
that case `IncrementalExtensions` would mean that we must turn on incremental 
processing for managing lifetime and only use the language option when 
extending the parsing logic. However, I think the problem would be what to do 
with the `tok::eof` and `tok::annot_repl_input_end`? I'd probably need 
@aaron.ballman or @rsmith here...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148997/new/

https://reviews.llvm.org/D148997

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148997: [clang] Add a new annotation token: annot_repl_input_end

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D148997#4561516 , @bnbarham wrote:

> In D148997#4561211 , @v.g.vassilev 
> wrote:
>
> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Parse/ParseExprCXX.cpp#L4070

 That looks a bit obscure to me. Looks like we are trying to reach some 
 error recovery anchor but do you happen to have some use case at hand? In 
 principle we could do the same as for the other 3.
>>>
>>> This was just one I picked at random from my grep over the parser. It's 
>>> unclear how often this would be hit, but I have to assume it (and the other 
>>> references) can, even if they're obscure/unlikely. My main point is that 
>>> `!eof` is being used somewhat frequently to end loops, but with this change 
>>> they will now all never end.
>>
>> Since `eof` there were several `eof`-like tokens that were added: 
>> `annot_module_begin`, `annot_module_end`, `tok::annot_module_include` which 
>> are commonly handled in `isEofOrEom`. We could update these uses with 
>> `isEofOrEom` but I am pretty sure that @rsmith will point out cases where 
>> that's not a good very good idea :) We could either experiment with using 
>> `isEofOrEom` or have a similar utility function only for the two tokens (say 
>> `isEoI` -- end of input whatever that means).
>
> My other concern here is that it seems our use cases are somewhat different, 
> eg. we wouldn't want any parsing differences and while I don't know why this 
> is yet, the removal of:

I think I understand now. We basically want a mode which keeps some of the 
clang objects alive and ready to process more input. And on top, for clang-repl 
we want to enable some special parsing mode for the top-level statement 
declarations.

>   // Skip over the EOF token, flagging end of previous input for incremental
>   // processing
>   if (PP.isIncrementalProcessingEnabled() && Tok.is(tok::eof))
> ConsumeToken();
>
> is causing issues for us as well.

Yes, that now seems to be moved on the user side as we thought it would be dead 
code. We can bring that one back if that's the only issue you see with that 
approach.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148997/new/

https://reviews.llvm.org/D148997

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148997: [clang] Add a new annotation token: annot_repl_input_end

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D148997#4561068 , @bnbarham wrote:

> In D148997#4561022 , @v.g.vassilev 
> wrote:
>
>> I meant that I'd like to figure out if we could use the 
>> `annot_repl_input_end` before considering a new flag.
>
> Oh sure, that's why I'm here :). Just trying to figure out what we think the 
> best way forward is.
>
>>> The issue is that all these actions (and the parser checks) can run with 
>>> and without `isIncrementalProcessingEnabled`, so they would need to check 
>>> both `eof` and `annot_repl_input_end`. For some concrete locations (but no 
>>> where near complete):
>>> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Frontend/FrontendActions.cpp#L82
>>> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Frontend/FrontendActions.cpp#L955
>>> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp#L542
>>
>> These three seem to be useful for `clang-repl` too, so we might want to 
>> extend it with like `!(eof || annot_repl_input_end)`
>
> Sure, though to be clear this isn't all of them, there's also the dump 
> actions too (though not sure that makes sense for `clang-repl`)

The dump actions are also somewhat interesting because in that case we will 
dump the delta that was accumulated and repeat.

and a few in other files (`VerifyDiagnosticConsumer.cpp`, 
`PrintPreprocessedOutput.cpp`).

These ones are also interesting.

>>> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Parse/ParseExprCXX.cpp#L4070
>>
>> That looks a bit obscure to me. Looks like we are trying to reach some error 
>> recovery anchor but do you happen to have some use case at hand? In 
>> principle we could do the same as for the other 3.
>
> This was just one I picked at random from my grep over the parser. It's 
> unclear how often this would be hit, but I have to assume it (and the other 
> references) can, even if they're obscure/unlikely. My main point is that 
> `!eof` is being used somewhat frequently to end loops, but with this change 
> they will now all never end.

Since `eof` there were several `eof`-like tokens that were added: 
`annot_module_begin`, `annot_module_end`, `tok::annot_module_include` which are 
commonly handled in `isEofOrEom`. We could update these uses with `isEofOrEom` 
but I am pretty sure that @rsmith will point out cases where that's not a good 
very good idea :) We could either experiment with using `isEofOrEom` or have a 
similar utility function only for the two tokens (say `isEoI` -- end of input 
whatever that means).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148997/new/

https://reviews.llvm.org/D148997

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148997: [clang] Add a new annotation token: annot_repl_input_end

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D148997#4561015 , @bnbarham wrote:

> In D148997#4559788 , @v.g.vassilev 
> wrote:
>
>> I'd prefer to avoid adding a new flag. Is there a way to see how does the 
>> diff looks like?
>
> You mean for a new flag? I don't have one prepared, but it would basically 
> just be adding an extra check where `isIncrementalProcessingEnabled` is 
> currently used to skip resetting `CurLexer` and `TUScope`. I don't believe 
> we'd want any difference in parsing in Swift's clang importer use case.

I meant that I'd like to figure out if we could use the `annot_repl_input_end` 
before considering a new flag.

>> Maybe it would make more sense to use the `annot_repl_input_end` token? If 
>> the token name does not capture well the generic use-case I am happy to 
>> change it to something better.
>
> The issue is that all these actions (and the parser checks) can run with and 
> without `isIncrementalProcessingEnabled`, so they would need to check both 
> `eof` and `annot_repl_input_end`. For some concrete locations (but no where 
> near complete):
> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Frontend/FrontendActions.cpp#L82
> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Frontend/FrontendActions.cpp#L955
> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp#L542

These three seem to be useful for `clang-repl` too, so we might want to extend 
it with like `!(eof || annot_repl_input_end)`

> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Parse/ParseExprCXX.cpp#L4070

That looks a bit obscure to me. Looks like we are trying to reach some error 
recovery anchor but do you happen to have some use case at hand? In principle 
we could do the same as for the other 3.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148997/new/

https://reviews.llvm.org/D148997

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148997: [clang] Add a new annotation token: annot_repl_input_end

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D148997#4559646 , @bnbarham wrote:

>> Are there other users of incremental processing mode, other than the REPL / 
>> IncrementalParser?
>
> It seems Swift's clang importer also uses incremental processing mode, I'm 
> assuming to keep the `TUScope` and `CurLexer` alive after EOF. We also end up 
> using the same context with various actions, which leads to a few hangs as 
> there's various checks for `eof` only, eg. `ReadPCHAndPreprocessAction`, 
> `PreprocessOnlyAction`, and `RewriteIncludesAction`. There's also quite a few 
> places in the parser that only check for `eof` as well (I just grepped for 
> `Tok.isNot(tok::eof)`).
>
> Should these all be updated to handle `annot_repl_input_end` or should we 
> instead have a different flag that we set for this purpose instead of 
> co-opting `isIncrementalProcessingEnabled`?

I'd prefer to avoid adding a new flag. Is there a way to see how does the diff 
looks like? Maybe it would make more sense to use the `annot_repl_input_end` 
token? If the token name does not capture well the generic use-case I am happy 
to change it to something better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148997/new/

https://reviews.llvm.org/D148997

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156877: Update Clang-REPL docs with removing the redundant subsections

[Vassil Vassilev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG54081868dd22: [clang-repl] Remove redundant subsections from 
the table of content. (authored by Krishna-13-cyber, committed by v.g.vassilev).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156877/new/

https://reviews.llvm.org/D156877

Files:
  clang/docs/ClangRepl.rst


Index: clang/docs/ClangRepl.rst
===
--- clang/docs/ClangRepl.rst
+++ clang/docs/ClangRepl.rst
@@ -47,7 +47,6 @@
 
 8. The machine code is then executed.
 
-===
 Build Instructions:
 ===
 
@@ -75,7 +74,6 @@
clang-repl>
 
 
-
 Clang-Repl Usage
 
 


Index: clang/docs/ClangRepl.rst
===
--- clang/docs/ClangRepl.rst
+++ clang/docs/ClangRepl.rst
@@ -47,7 +47,6 @@
 
 8. The machine code is then executed.
 
-===
 Build Instructions:
 ===
 
@@ -75,7 +74,6 @@
clang-repl>
 
 
-
 Clang-Repl Usage
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/docs/index.rst:96
ClangRepl
+   ExecutionResultsHandling
 

We should probably move that under `ClangRepl`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156877: Update Clang-REPL docs with removing the redundant subsections

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156877/new/

https://reviews.llvm.org/D156877

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D156537#4554052 , @Hahnfeld wrote:

> @v.g.vassilev do we have a means to detect this case? In D156897 
> , I'm refactoring all accesses to 
> `EmittedDeferredDecl` to go via `addEmittedDeferredDecl`, so we should just 
> add an early return when not emitting for a REPL.

We have `getLangOpts().IncrementalExtensions`. Does that work?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156537/new/

https://reviews.llvm.org/D156537

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

I think we are getting there. I added a few suggestions how to improve the 
current design.




Comment at: clang/include/clang/Interpreter/CodeCompletion.h:15
+#define LLVM_CLANG_INTERPRETER_CODE_COMPLETION_H
+#include "llvm/LineEditor/LineEditor.h"
+

I still am a bit uncomfortable with relying on a line editor api to address a 
generic question one could as the interpreter such as "What's visible at that 
code position"?

Can we not make the overloaded operators outside of the `ReplListCompleter` 
class. That means moving them in ClangRepl.cpp?



Comment at: clang/include/clang/Interpreter/Interpreter.h:42
 class IncrementalParser;
+class CodeCompleteConsumer;
 

We should put that in its alphabetical order.



Comment at: clang/lib/Interpreter/CodeCompletion.cpp:111
+  auto *CConsumer = new ReplCompletionConsumer(Results);
+  auto Interp = Interpreter::createForCodeCompletion(
+  CB, MainInterp.getCompilerInstance(), CConsumer);

I do not believe we need a spe



Comment at: clang/lib/Interpreter/IncrementalParser.cpp:324
 
-llvm::Expected
-IncrementalParser::Parse(llvm::StringRef input) {
+llvm::Error IncrementalParser::ParseForCodeCompletion(llvm::StringRef input,
+   size_t Col, size_t Line) {

Is there any other particular reason for this routine to be different rather 
than `PP.SetCodeCompletionPoint(*FE, Line, Col)`? That is, can we merge back 
the old code and only set the point of completion externally?



Comment at: clang/lib/Interpreter/Interpreter.cpp:304
+llvm::Expected>
+Interpreter::createForCodeCompletion(
+IncrementalCompilerBuilder , const CompilerInstance *ParentCI,

capfredf wrote:
> v.g.vassilev wrote:
> > I still do not entirely understand why we can "just" ask the codecompletion 
> > infrastructure for the possible options at the current position. I know 
> > that's probably easier set than done but I'd like to entertain that idea 
> > for a while..
> I'm a bit confused. Can you expand on the suggestion?
I do not believe we need a special case for creating a CodeCompletionConsumer. 
I think we could rely on the logic in 
`CompilerInstance::createCodeCompletionConsumer` which can be triggered in 
`IncrementalAction::ExecuteAction` just like in 
`ASTFrontendAction::ExecuteAction`. We will need to copy some of the code over 
but then you could start the same interpreter infrastructure with slightly 
different flags.



Comment at: clang/lib/Parse/ParseDecl.cpp:6652
+break;
+  }
+

Can we test this change separately outside of clang-repl? That would mean 
creating a tests and run it with `clang -Xclang -fincremental-extensions`



Comment at: clang/lib/Parse/Parser.cpp:934
+  PCC = Sema::PCC_Namespace;
+};
 Actions.CodeCompleteOrdinaryName(

Likewise, it would be great if we have a test in clang-only mode.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156660: [CodeGen] Assert that EmittedDeferredDecls is empty

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156660/new/

https://reviews.llvm.org/D156660

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156425: [clang-repl] Remove redundant tests

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156425/new/

https://reviews.llvm.org/D156425

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a subscriber: alexfh.
v.g.vassilev added a comment.

@alexfh, could you test that patch on your internal infrastructure and check if 
it actually decreases the memory footprint and/or compile times? I believe this 
was the only blocker to land it as it is.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D154324#4524368 , @rsmith wrote:

> In D154324#4524235 , @v.g.vassilev 
> wrote:
>
>> @rsmith, thanks for the suggestions! Could you go over 
>> `ODRHash::AddTemplateName` suggest how to fix it to address 
>> https://reviews.llvm.org/D153003 and https://reviews.llvm.org/D41416#4496451?
>
> `AddTemplateName` looks fine as-is to me; I think the problem in D153003 
>  is that we'd stepped outside of the entity 
> we were odr-hashing and started hashing something else, which (legitimately) 
> was different between translation units.
>
> For D41416 , ODR hashing may not be the best 
> mechanism to hash the template arguments, unfortunately. ODR hashing is (or 
> perhaps, should be) about determining whether two things are spelled the same 
> way and have the same meaning (as required by the C++ ODR), whereas I think 
> what you're looking for is whether they have the same meaning regardless of 
> spelling. Maybe we can get away with reusing ODR hashing anyway, on the basis 
> that any canonical, non-dependent template argument should have the same 
> (invented) spelling in every translation unit, but I'm not certain that's 
> true in all cases. There may still be cases where the canonical type includes 
> some aspect of "whatever we saw first", in which case the ODR hash can differ 
> across translation units for non-dependent, canonical template arguments that 
> are spelled differently but have the same meaning, though I can't think of 
> one off-hand.

Thanks for investigating. I am happy to try to get away with (mis)using ODR 
hashing and see if we (and for how long) could get away with it. @Hahnfeld and 
I discussed to use the llvm FoldingSet technique if ODR hash falls short. Is 
that or it was something else what you had in mind as an alternative to ODR 
hashing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154324/new/

https://reviews.llvm.org/D154324

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

@rsmith, thanks for the suggestions! Could you go over 
`ODRHash::AddTemplateName` suggest how to fix it to address 
https://reviews.llvm.org/D153003 and https://reviews.llvm.org/D41416#4496451?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154324/new/

https://reviews.llvm.org/D154324

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/unittests/Interpreter/CodeCompletionTest.cpp:71
+Completer(std::string("void app("), 9);
+  EXPECT_EQ((size_t)0, out.length());
+}

capfredf wrote:
> @v.g.vassilev The way I fixed this is a bit hacky. Do you have a better idea?
Now I think I understand what you were after. Instead of handling the error in 
the constructor we should handle it in the caller. Here is some information how 
we could do that: 
https://llvm.org/docs/ProgrammersManual.html#fallible-constructors


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146809: [clang-repl] Implement pretty printing of custom types.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 541061.
v.g.vassilev added a comment.

Add forgotten file.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146809/new/

https://reviews.llvm.org/D146809

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/PartialTranslationUnit.h
  clang/include/clang/Interpreter/Value.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_interpreter_runtime_printvalue.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/DeviceOffload.cpp
  clang/lib/Interpreter/DeviceOffload.h
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Interpreter/ValuePrinter.cpp
  clang/test/Interpreter/pretty-print.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/IncrementalProcessingTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -29,6 +29,7 @@
 #include "gtest/gtest.h"
 
 using namespace clang;
+using namespace caas;
 
 #if defined(_AIX)
 #define CLANG_INTERPRETER_NO_SUPPORT_EXEC
@@ -46,12 +47,12 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CB = clang::IncrementalCompilerBuilder();
+  auto CB = clang::caas::IncrementalCompilerBuilder();
   CB.SetCompilerArgs(ClangArgs);
   auto CI = cantFail(CB.CreateCpp());
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
-  return cantFail(clang::Interpreter::create(std::move(CI)));
+  return cantFail(clang::caas::Interpreter::create(std::move(CI)));
 }
 
 static size_t DeclsSize(TranslationUnitDecl *PTUDecl) {
Index: clang/unittests/Interpreter/IncrementalProcessingTest.cpp
===
--- clang/unittests/Interpreter/IncrementalProcessingTest.cpp
+++ clang/unittests/Interpreter/IncrementalProcessingTest.cpp
@@ -27,6 +27,7 @@
 
 using namespace llvm;
 using namespace clang;
+using namespace clang::caas;
 
 namespace {
 
@@ -52,12 +53,12 @@
 
 TEST(IncrementalProcessing, EmitCXXGlobalInitFunc) {
   std::vector ClangArgv = {"-Xclang", "-emit-llvm-only"};
-  auto CB = clang::IncrementalCompilerBuilder();
+  auto CB = clang::caas::IncrementalCompilerBuilder();
   CB.SetCompilerArgs(ClangArgv);
   auto CI = cantFail(CB.CreateCpp());
   auto Interp = llvm::cantFail(Interpreter::create(std::move(CI)));
 
-  std::array PTUs;
+  std::array PTUs;
 
   PTUs[0] = ::cantFail(Interp->Parse(TestProgram1));
   ASSERT_TRUE(PTUs[0]->TheModule);
Index: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
===
--- clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -30,6 +30,7 @@
 #include "gtest/gtest.h"
 
 using namespace clang;
+using namespace clang::caas;
 
 namespace {
 using Args = std::vector;
@@ -38,12 +39,12 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CB = clang::IncrementalCompilerBuilder();
+  auto CB = clang::caas::IncrementalCompilerBuilder();
   CB.SetCompilerArgs(ClangArgs);
   auto CI = cantFail(CB.CreateCpp());
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
-  return cantFail(clang::Interpreter::create(std::move(CI)));
+  return cantFail(clang::caas::Interpreter::create(std::move(CI)));
 }
 
 TEST(InterpreterTest, CatchException) {
Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -97,7 +97,7 @@
 return 0;
   }
 
-  clang::IncrementalCompilerBuilder CB;
+  clang::caas::IncrementalCompilerBuilder CB;
   CB.SetCompilerArgs(ClangArgv);
 
   std::unique_ptr DeviceCI;
@@ -132,10 +132,10 @@
   if (CudaEnabled)
 DeviceCI->LoadRequestedPlugins();
 
-  std::unique_ptr Interp;
+  std::unique_ptr Interp;
   if (CudaEnabled) {
-Interp = ExitOnErr(
-clang::Interpreter::createWithCUDA(std::move(CI), std::move(DeviceCI)));
+Interp = 

[PATCH] D146809: [clang-repl] Implement pretty printing of custom types.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Headers/__clang_interpreter_runtime_printvalue.h:79
+
+template  struct is_iterable : std::false_type {};
+

aaron.ballman wrote:
> Still need to make sure you're using reserved identifiers for these (should 
> make a pass through the file and get all of the identifiers).
Could you check now, I think I fixed all I thought needed fixing.



Comment at: clang/lib/Interpreter/ValuePrinter.cpp:164
+
+// TODO: Encodings.
+static std::string PrintOneChar(char Val) {

aaron.ballman wrote:
> Not just encodings, but also: how do you handle unprintable characters (like 
> control characters)?
Extended the FIXME.



Comment at: clang/lib/Interpreter/ValuePrinter.cpp:543
+default:
+  llvm_unreachable("Unknown Builtintype kind");
+}

aaron.ballman wrote:
> This seems rather reachable, no?
Do you mean that's not exhaustive list that we list here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146809/new/

https://reviews.llvm.org/D146809

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146809: [clang-repl] Implement pretty printing of custom types.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 541048.
v.g.vassilev marked 5 inline comments as done.
v.g.vassilev retitled this revision from "[clang-repl] Implement Value pretty 
printing" to "[clang-repl] Implement pretty printing of custom types.".
v.g.vassilev added a comment.
Herald added a subscriber: jvesely.

- Address several comments
- Make __clang_interpreter_runtime_printvalue.h C++11 compliant.
- Support std::vector
- Add more tests


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146809/new/

https://reviews.llvm.org/D146809

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/PartialTranslationUnit.h
  clang/include/clang/Interpreter/Value.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_interpreter_runtime_printvalue.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/DeviceOffload.cpp
  clang/lib/Interpreter/DeviceOffload.h
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/test/Interpreter/pretty-print.cpp
  clang/test/Modules/pr60085.cppm
  clang/tools/clang-repl/CMakeLists.txt
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/IncrementalProcessingTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -29,6 +29,7 @@
 #include "gtest/gtest.h"
 
 using namespace clang;
+using namespace caas;
 
 #if defined(_AIX)
 #define CLANG_INTERPRETER_NO_SUPPORT_EXEC
@@ -46,12 +47,12 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CB = clang::IncrementalCompilerBuilder();
+  auto CB = clang::caas::IncrementalCompilerBuilder();
   CB.SetCompilerArgs(ClangArgs);
   auto CI = cantFail(CB.CreateCpp());
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
-  return cantFail(clang::Interpreter::create(std::move(CI)));
+  return cantFail(clang::caas::Interpreter::create(std::move(CI)));
 }
 
 static size_t DeclsSize(TranslationUnitDecl *PTUDecl) {
Index: clang/unittests/Interpreter/IncrementalProcessingTest.cpp
===
--- clang/unittests/Interpreter/IncrementalProcessingTest.cpp
+++ clang/unittests/Interpreter/IncrementalProcessingTest.cpp
@@ -27,6 +27,7 @@
 
 using namespace llvm;
 using namespace clang;
+using namespace clang::caas;
 
 namespace {
 
@@ -52,12 +53,12 @@
 
 TEST(IncrementalProcessing, EmitCXXGlobalInitFunc) {
   std::vector ClangArgv = {"-Xclang", "-emit-llvm-only"};
-  auto CB = clang::IncrementalCompilerBuilder();
+  auto CB = clang::caas::IncrementalCompilerBuilder();
   CB.SetCompilerArgs(ClangArgv);
   auto CI = cantFail(CB.CreateCpp());
   auto Interp = llvm::cantFail(Interpreter::create(std::move(CI)));
 
-  std::array PTUs;
+  std::array PTUs;
 
   PTUs[0] = ::cantFail(Interp->Parse(TestProgram1));
   ASSERT_TRUE(PTUs[0]->TheModule);
Index: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
===
--- clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -30,6 +30,7 @@
 #include "gtest/gtest.h"
 
 using namespace clang;
+using namespace clang::caas;
 
 namespace {
 using Args = std::vector;
@@ -38,12 +39,12 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CB = clang::IncrementalCompilerBuilder();
+  auto CB = clang::caas::IncrementalCompilerBuilder();
   CB.SetCompilerArgs(ClangArgs);
   auto CI = cantFail(CB.CreateCpp());
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
-  return cantFail(clang::Interpreter::create(std::move(CI)));
+  return cantFail(clang::caas::Interpreter::create(std::move(CI)));
 }
 
 TEST(InterpreterTest, CatchException) {
Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -97,7 +97,7 @@
 return 0;
   }
 
-  clang::IncrementalCompilerBuilder CB;
+  clang::caas::IncrementalCompilerBuilder CB;
   CB.SetCompilerArgs(ClangArgv);
 
   

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D41416#4496389 , @ChuanqiXu wrote:

> In D41416#4496293 , @v.g.vassilev 
> wrote:
>
>> In D41416#4492367 , @v.g.vassilev 
>> wrote:
>>
>>> Address most of the comments. I will need some help with the 
>>> `Modules/pr60085.cppm` failure. I suspect we pass to CodeGen some 
>>> instantiation by iterator index and that does not work as the 
>>> instantiations are lazily triggered upon use now.
>>
>> In fact it looks like it fails exactly in the same way as described in the 
>> original report https://github.com/llvm/llvm-project/issues/60085. The 
>> commit message in https://github.com/llvm/llvm-project/commit/78e48977a6e67 
>> hints at the fact that the issue was gone surprisingly. I suspect that test 
>> case stopped reproducing the underlying issue by chance. That probably means 
>> that this patch is not breaking anything but exposing an underlying 
>> problem...
>
> Got it. I understand it is frustrating to fix an additional bug during 
> development. Also it is generally not good to revert a valid test case.

I agree. Here is a reduced version of the same test case:

  // RUN: rm -rf %t
  // RUN: mkdir %t
  // RUN: split-file %s %t
  //
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/d.cppm \
  // RUN: -emit-module-interface -o %t/d.pcm
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \
  // RUN: -emit-module-interface -o %t/c.pcm -fmodule-file=%t/d.pcm
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \
  // RUN: -emit-module-interface -o %t/b.pcm -fmodule-file=%t/d.pcm
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \
  // RUN: -emit-module-interface -o %t/a.pcm -fmodule-file=%t/d.pcm \
  // RUN: -fmodule-file=%t/c.pcm -fmodule-file=%t/b.pcm 
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.pcm \
  // RUN: -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/a.cppm
  //
  // Use -fmodule-file==
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/d.cppm \
  // RUN: -emit-module-interface -o %t/d.pcm
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \
  // RUN: -emit-module-interface -o %t/c.pcm -fmodule-file=%t/d.pcm
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \
  // RUN: -emit-module-interface -o %t/b.pcm -fmodule-file=%t/d.pcm
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \
  // RUN: -emit-module-interface -o %t/a.pcm -fmodule-file=%t/d.pcm \
  // RUN: -fmodule-file=%t/c.pcm -fmodule-file=%t/b.pcm 
  // RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.pcm \
  // RUN: -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/a.cppm
  
  //--- d.cppm
  export module d;
  
  export template
  struct integer {
using type = int;
  
static constexpr auto value() {
  return 0;
}
  
  };
  
  export constexpr void ddd(auto const v) {
v.value();
  }
  
  //--- c.cppm
  export module c;
  
  import d;
  
  int use = integer().value();
  
  //--- b.cppm
  export module b;
  
  import d;
  
  using use = integer::type;
  
  //--- a.cppm
  export module a;
  
  import d;
  
  export void a() {
ddd(integer());
  }



> Are you in a hurry to land this recently? (e.g., land this for clang17) If 
> not, I suggest to wait for me to fix the underlying issue after clang17 gets 
> released. And if I take too long to fix it (e.g, 9.30), given the speciality 
> of the patch, I think we can revert that test case and land this one that 
> time. How do you feel about the idea?

We are not in hurry - this patch was developed in 2017 :) It can probably wait 
a few more years ;) I had some spare cycles to work on it. That's all.

I do not think we can land it without asking @rsmith or @dblaikie to test it on 
their internal builds to make sure things are not regressing performance...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D41416#4492367 , @v.g.vassilev 
wrote:

> Address most of the comments. I will need some help with the 
> `Modules/pr60085.cppm` failure. I suspect we pass to CodeGen some 
> instantiation by iterator index and that does not work as the instantiations 
> are lazily triggered upon use now.

In fact it looks like it fails exactly in the same way as described in the 
original report https://github.com/llvm/llvm-project/issues/60085. The commit 
message in https://github.com/llvm/llvm-project/commit/78e48977a6e67 hints at 
the fact that the issue was gone surprisingly. I suspect that test case stopped 
reproducing the underlying issue by chance. That probably means that this patch 
is not breaking anything but exposing an underlying problem...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 539670.
v.g.vassilev added a comment.

Fix the odr_hash.cpp failure.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/ODRHash.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/cxx-templates.cpp
  clang/test/Modules/odr_hash.cpp

Index: clang/test/Modules/odr_hash.cpp
===
--- clang/test/Modules/odr_hash.cpp
+++ clang/test/Modules/odr_hash.cpp
@@ -2901,8 +2901,8 @@
 };
 #else
 S5 s5;
-// expected-error@second.h:* {{'PointersAndReferences::S5::x' from module 'SecondModule' is not present in definition of 'PointersAndReferences::S5' in module 'FirstModule'}}
-// expected-note@first.h:* {{declaration of 'x' does not match}}
+// expected-error@first.h:* {{'PointersAndReferences::S5::x' from module 'FirstModule' is not present in definition of 'PointersAndReferences::S5' in module 'SecondModule'}}
+// expected-note@second.h:* {{declaration of 'x' does not match}}
 #endif
 
 #if defined(FIRST)
Index: clang/test/Modules/cxx-templates.cpp
===
--- clang/test/Modules/cxx-templates.cpp
+++ clang/test/Modules/cxx-templates.cpp
@@ -251,7 +251,7 @@
 
 // CHECK-DUMP:  ClassTemplateDecl {{.*}} <{{.*[/\\]}}cxx-templates-common.h:1:1, {{.*}}>  col:{{.*}} in cxx_templates_common SomeTemplate
 // CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
 // CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
 // CHECK-DUMP-NEXT: DefinitionData
 // CHECK-DUMP-NEXT:   DefaultConstructor
@@ -260,9 +260,9 @@
 // CHECK-DUMP-NEXT:   CopyAssignment
 // CHECK-DUMP-NEXT:   MoveAssignment
 // CHECK-DUMP-NEXT:   Destructor
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
-// CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
 // CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
+// CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
 // CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
 // CHECK-DUMP-NEXT: DefinitionData
 // CHECK-DUMP-NEXT:   DefaultConstructor
@@ -271,4 +271,4 @@
 // CHECK-DUMP-NEXT:   CopyAssignment
 // CHECK-DUMP-NEXT:   MoveAssignment
 // CHECK-DUMP-NEXT:   Destructor
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -177,11 +177,12 @@
   Record.AddSourceLocation(typeParams->getRAngleLoc());
 }
 
-/// Add to the record the first declaration from each module file that
-/// provides a declaration of D. The intent is to provide a sufficient
-/// set such that reloading this set will load all current redeclarations.
-void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
-  llvm::MapVector Firsts;
+/// Collect the first declaration from each module file that provides a
+/// declaration of D.
+void CollectFirstDeclFromEachModule(
+const Decl *D, bool IncludeLocal,
+llvm::MapVector ) {
+
   // FIXME: We can skip entries that we know are implied by others.
   for (const Decl *R = D->getMostRecentDecl(); R; R = R->getPreviousDecl()) {
 if (R->isFromASTFile())
@@ -189,10 +190,49 @@
 else if (IncludeLocal)
   Firsts[nullptr] = R;
   }
+}
+
+/// Add to the record the first declaration from each module file that
+/// provides a declaration of D. The intent is to provide a sufficient
+/// set such that reloading this set will load all current redeclarations.
+void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
+  llvm::MapVector Firsts;
+  CollectFirstDeclFromEachModule(D, IncludeLocal, Firsts);
+
   for (const auto  : Firsts)
 Record.AddDeclRef(F.second);
 }
 
+/// Add to the record the first template specialization from each module
+/// file that provides a declaration of D. We store the DeclId and an
+/// ODRHash of the template arguments of D which should provide enough
+/// information to load D only if the template instantiator needs it.
+void AddFirstSpecializationDeclFromEachModule(const Decl *D,
+  

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 539664.
v.g.vassilev added a comment.

Fix a compilation error.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/ODRHash.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/cxx-templates.cpp

Index: clang/test/Modules/cxx-templates.cpp
===
--- clang/test/Modules/cxx-templates.cpp
+++ clang/test/Modules/cxx-templates.cpp
@@ -251,7 +251,7 @@
 
 // CHECK-DUMP:  ClassTemplateDecl {{.*}} <{{.*[/\\]}}cxx-templates-common.h:1:1, {{.*}}>  col:{{.*}} in cxx_templates_common SomeTemplate
 // CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
 // CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
 // CHECK-DUMP-NEXT: DefinitionData
 // CHECK-DUMP-NEXT:   DefaultConstructor
@@ -260,9 +260,9 @@
 // CHECK-DUMP-NEXT:   CopyAssignment
 // CHECK-DUMP-NEXT:   MoveAssignment
 // CHECK-DUMP-NEXT:   Destructor
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
-// CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
 // CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
+// CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
 // CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
 // CHECK-DUMP-NEXT: DefinitionData
 // CHECK-DUMP-NEXT:   DefaultConstructor
@@ -271,4 +271,4 @@
 // CHECK-DUMP-NEXT:   CopyAssignment
 // CHECK-DUMP-NEXT:   MoveAssignment
 // CHECK-DUMP-NEXT:   Destructor
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -177,11 +177,12 @@
   Record.AddSourceLocation(typeParams->getRAngleLoc());
 }
 
-/// Add to the record the first declaration from each module file that
-/// provides a declaration of D. The intent is to provide a sufficient
-/// set such that reloading this set will load all current redeclarations.
-void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
-  llvm::MapVector Firsts;
+/// Collect the first declaration from each module file that provides a
+/// declaration of D.
+void CollectFirstDeclFromEachModule(
+const Decl *D, bool IncludeLocal,
+llvm::MapVector ) {
+
   // FIXME: We can skip entries that we know are implied by others.
   for (const Decl *R = D->getMostRecentDecl(); R; R = R->getPreviousDecl()) {
 if (R->isFromASTFile())
@@ -189,10 +190,49 @@
 else if (IncludeLocal)
   Firsts[nullptr] = R;
   }
+}
+
+/// Add to the record the first declaration from each module file that
+/// provides a declaration of D. The intent is to provide a sufficient
+/// set such that reloading this set will load all current redeclarations.
+void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
+  llvm::MapVector Firsts;
+  CollectFirstDeclFromEachModule(D, IncludeLocal, Firsts);
+
   for (const auto  : Firsts)
 Record.AddDeclRef(F.second);
 }
 
+/// Add to the record the first template specialization from each module
+/// file that provides a declaration of D. We store the DeclId and an
+/// ODRHash of the template arguments of D which should provide enough
+/// information to load D only if the template instantiator needs it.
+void AddFirstSpecializationDeclFromEachModule(const Decl *D,
+  bool IncludeLocal) {
+  assert(isa(D) ||
+ isa(D) ||
+ isa(D) && "Must not be called with other decls");
+  llvm::MapVector Firsts;
+  CollectFirstDeclFromEachModule(D, IncludeLocal, Firsts);
+
+  for (const auto  : Firsts) {
+Record.AddDeclRef(F.second);
+ArrayRef Args;
+if (auto *CTSD = dyn_cast(D))
+  Args = CTSD->getTemplateArgs().asArray();
+else if (auto *VTSD = dyn_cast(D))
+  Args = VTSD->getTemplateArgs().asArray();
+else if (auto *FD = dyn_cast(D))
+  Args = FD->getTemplateSpecializationArgs()->asArray();
+assert(Args.size());
+Record.push_back(TemplateArgumentList::ComputeODRHash(Args));
+bool IsPartialSpecialization =
+  

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/include/clang/Sema/CodeCompleteConsumer.h:342
+/// Code completion at a top level in a REPL session.
+CCC_ReplTopLevel,
   };





Comment at: clang/lib/Interpreter/CodeCompletion.cpp:102
+  std::vector Results;
+  auto *CConsumer = new ReplCompletionConsumer(Results);
+  auto Interp = Interpreter::createForCodeCompletion(

Let's move this in `Interpreter::createForCodeCompletion`.



Comment at: clang/lib/Interpreter/CodeCompletion.cpp:118
+  llvm::StringRef s;
+  if (space_pos == llvm::StringRef::npos) {
+s = Buffer;

Please remove the braces around single statement blocks.



Comment at: clang/lib/Interpreter/Interpreter.cpp:304
+llvm::Expected>
+Interpreter::createForCodeCompletion(
+IncrementalCompilerBuilder , const CompilerInstance *ParentCI,

I still do not entirely understand why we can "just" ask the codecompletion 
infrastructure for the possible options at the current position. I know that's 
probably easier set than done but I'd like to entertain that idea for a while..


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146809: [clang-repl] Implement Value pretty printing

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev commandeered this revision.
v.g.vassilev edited reviewers, added: junaire; removed: v.g.vassilev.
v.g.vassilev added a comment.

Jun has moved on in his career and I'd like to take responsibility for this 
patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146809/new/

https://reviews.llvm.org/D146809

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a reviewer: aaron.ballman.
v.g.vassilev added a subscriber: aaron.ballman.
v.g.vassilev added a comment.

@capfredf this seems to be heading in a good direction. Please find another 
round of comments.

@aaron.ballman, I could not find a person that's active in that area. Could you 
help with finding reviewers for this type of patch?




Comment at: clang/include/clang/Interpreter/Interpreter.h:117
   ASTContext ();
+  void CodeComplete(llvm::StringRef Input, size_t Col, size_t Line = 1);
   const CompilerInstance *getCompilerInstance() const;

I could not find where this interface was used. If it is unused, let'd drop it.



Comment at: clang/include/clang/Sema/Sema.h:13324
+/// Code completion occurs at top-level in a REPL session
+PCC_ReplTopLevel,
   };





Comment at: clang/lib/Interpreter/IncrementalParser.cpp:338
+  //   return;
+  // }
+  if (FE) {

Seems that we have stray debug fragments? Can you remove them?



Comment at: clang/lib/Interpreter/IncrementalParser.cpp:376
   // Create FileID for the current buffer.
-  FileID FID = SM.createFileID(std::move(MB), SrcMgr::C_User, /*LoadedID=*/0,
-   /*LoadedOffset=*/0, NewLoc);
+  // FileID FID = SM.createFileID(std::move(MB), SrcMgr::C_User, 
/*LoadedID=*/0,
+  //  /*LoadedOffset=*/0, NewLoc);

Why we had to comment out this line?



Comment at: clang/lib/Interpreter/Interpreter.cpp:248
+  auto *CConsumer = new ReplCompletionConsumer(CompResults);
+  CI->setCodeCompletionConsumer(CConsumer);
+  IncrParser = std::make_unique(

IIUC, `setCodeCompletionConsumer` takes the ownership of `CConsumer`. I'd 
suggest to drop `CConsumer` member.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/test/Modules/odr_hash.cpp:2905
 // expected-error@second.h:* {{'PointersAndReferences::S5::x' from module 
'SecondModule' is not present in definition of 'PointersAndReferences::S5' in 
module 'FirstModule'}}
-// expected-note@first.h:* {{declaration of 'x' does not match}}
 #endif

I realize that change is probably incorrect since we want to show both places 
where things differ.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/AST/DeclTemplate.cpp:337
+void RedeclarableTemplateDecl::loadLazySpecializationsImpl(
+ bool OnlyPartial/*=false*/) const 
{
   // Grab the most recent declaration to ensure we've loaded any lazy

shafik wrote:
> `/*=false*/` is this left over code that was meant to be removed?
That was not my intent. I added `/*=false*/` to keep track of the the actual 
default argument.

However, you are right, we are not using it without passing the correct 
argument. I will drop the default arg.



Comment at: clang/lib/AST/DeclTemplate.cpp:366
+  Args,
+  TemplateParameterList 
*TPL) const {
+  CommonBase *CommonBasePtr = getMostRecentDecl()->getCommonPtr();

Hahnfeld wrote:
> ChuanqiXu wrote:
> > TPL here looks not used.
> This is required because of the argument forwarding from 
> `findSpecializationImpl` below...
Are you suggesting to make this replacement: `TPL` -> `/*TPL*/`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 539409.
v.g.vassilev marked 7 inline comments as done.
v.g.vassilev added a comment.

Address most of the comments. I will need some help with the 
`Modules/pr60085.cppm` failure. I suspect we pass to CodeGen some instantiation 
by iterator index and that does not work as the instantiations are lazily 
triggered upon use now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/ODRHash.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/cxx-templates.cpp
  clang/test/Modules/odr_hash.cpp

Index: clang/test/Modules/odr_hash.cpp
===
--- clang/test/Modules/odr_hash.cpp
+++ clang/test/Modules/odr_hash.cpp
@@ -2902,7 +2902,7 @@
 #else
 S5 s5;
 // expected-error@second.h:* {{'PointersAndReferences::S5::x' from module 'SecondModule' is not present in definition of 'PointersAndReferences::S5' in module 'FirstModule'}}
-// expected-note@first.h:* {{declaration of 'x' does not match}}
+// expected-note@second.h:* {{declaration of 'x' does not match}}
 #endif
 
 #if defined(FIRST)
@@ -3839,7 +3839,7 @@
 #else
 Invalid::L2<1>::L3<1> invalid;
 // expected-error@second.h:* {{'Types::InjectedClassName::Invalid::L2::L3::x' from module 'SecondModule' is not present in definition of 'L3<>' in module 'FirstModule'}}
-// expected-note@first.h:* {{declaration of 'x' does not match}}
+// expected-note@second.h:* {{declaration of 'x' does not match}}
 Valid::L2<1>::L3<1> valid;
 #endif
 }  // namespace InjectedClassName
Index: clang/test/Modules/cxx-templates.cpp
===
--- clang/test/Modules/cxx-templates.cpp
+++ clang/test/Modules/cxx-templates.cpp
@@ -251,7 +251,7 @@
 
 // CHECK-DUMP:  ClassTemplateDecl {{.*}} <{{.*[/\\]}}cxx-templates-common.h:1:1, {{.*}}>  col:{{.*}} in cxx_templates_common SomeTemplate
 // CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
 // CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
 // CHECK-DUMP-NEXT: DefinitionData
 // CHECK-DUMP-NEXT:   DefaultConstructor
@@ -260,9 +260,9 @@
 // CHECK-DUMP-NEXT:   CopyAssignment
 // CHECK-DUMP-NEXT:   MoveAssignment
 // CHECK-DUMP-NEXT:   Destructor
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
-// CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
 // CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
+// CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
 // CHECK-DUMP:ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
 // CHECK-DUMP-NEXT: DefinitionData
 // CHECK-DUMP-NEXT:   DefaultConstructor
@@ -271,4 +271,4 @@
 // CHECK-DUMP-NEXT:   CopyAssignment
 // CHECK-DUMP-NEXT:   MoveAssignment
 // CHECK-DUMP-NEXT:   Destructor
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -177,11 +177,12 @@
   Record.AddSourceLocation(typeParams->getRAngleLoc());
 }
 
-/// Add to the record the first declaration from each module file that
-/// provides a declaration of D. The intent is to provide a sufficient
-/// set such that reloading this set will load all current redeclarations.
-void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
-  llvm::MapVector Firsts;
+/// Collect the first declaration from each module file that provides a
+/// declaration of D.
+void CollectFirstDeclFromEachModule(
+const Decl *D, bool IncludeLocal,
+llvm::MapVector ) {
+
   // FIXME: We can skip entries that we know are implied by others.
   for (const Decl *R = D->getMostRecentDecl(); R; R = R->getPreviousDecl()) {
 if (R->isFromASTFile())
@@ -189,10 +190,49 @@
 else if (IncludeLocal)
   Firsts[nullptr] = R;
   }
+}
+
+/// Add to the record the first declaration from each module file that
+/// provides a declaration of D. The intent is to provide a sufficient
+/// set such that reloading this set will load all current redeclarations.
+void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
+  llvm::MapVector Firsts;
+  CollectFirstDeclFromEachModule(D, 

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:288
+  }
+  for (auto  : LazySpecializations) {
+Record.push_back(SpecInfo.DeclID);

SAtacker wrote:
> ChuanqiXu wrote:
> > v.g.vassilev wrote:
> > > We should not store the lazy specialization information as an array of 
> > > items because that makes the search slow. Instead we should use the 
> > > `OnDiskHashTable` approach which we use already to store the identifier 
> > > data.
> > Do you want to implement it in this patch? Or this is a note for future 
> > optimizations?
> I tried it here https://reviews.llvm.org/D144831 but to my surprise it gives 
> worse performance for my testing environment which generates random 
> specializations per module and a single main file.
> Do you want to implement it in this patch? Or this is a note for future 
> optimizations?

That was the intent since it was blocking the google folks. If that's not the 
case then maybe we can do it as a separate improvement...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 538633.
v.g.vassilev added a comment.

Rebase.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41416/new/

https://reviews.llvm.org/D41416

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/ODRHash.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp

Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -177,11 +177,11 @@
   Record.AddSourceLocation(typeParams->getRAngleLoc());
 }
 
-/// Add to the record the first declaration from each module file that
-/// provides a declaration of D. The intent is to provide a sufficient
-/// set such that reloading this set will load all current redeclarations.
-void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
-  llvm::MapVector Firsts;
+/// Collect the first declaration from each module file that provides a
+/// declaration of D.
+void CollectFirstDeclFromEachModule(const Decl *D, bool IncludeLocal,
+llvm::MapVector ) {
+
   // FIXME: We can skip entries that we know are implied by others.
   for (const Decl *R = D->getMostRecentDecl(); R; R = R->getPreviousDecl()) {
 if (R->isFromASTFile())
@@ -189,10 +189,49 @@
 else if (IncludeLocal)
   Firsts[nullptr] = R;
   }
+}
+
+/// Add to the record the first declaration from each module file that
+/// provides a declaration of D. The intent is to provide a sufficient
+/// set such that reloading this set will load all current redeclarations.
+void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
+  llvm::MapVector Firsts;
+  CollectFirstDeclFromEachModule(D, IncludeLocal, Firsts);
+
   for (const auto  : Firsts)
 Record.AddDeclRef(F.second);
 }
 
+/// Add to the record the first template specialization from each module
+/// file that provides a declaration of D. We store the DeclId and an
+/// ODRHash of the template arguments of D which should provide enough
+/// information to load D only if the template instantiator needs it.
+void AddFirstSpecializationDeclFromEachModule(const Decl *D,
+  bool IncludeLocal) {
+  assert(isa(D) ||
+ isa(D) || isa(D) &&
+ "Must not be called with other decls");
+  llvm::MapVector Firsts;
+  CollectFirstDeclFromEachModule(D, IncludeLocal, Firsts);
+
+  for (const auto  : Firsts) {
+Record.AddDeclRef(F.second);
+ArrayRef Args;
+if (auto *CTSD = dyn_cast(D))
+  Args = CTSD->getTemplateArgs().asArray();
+else if (auto *VTSD = dyn_cast(D))
+  Args = VTSD->getTemplateArgs().asArray();
+else if (auto *FD = dyn_cast(D))
+  Args = FD->getTemplateSpecializationArgs()->asArray();
+assert(Args.size());
+Record.push_back(TemplateArgumentList::ComputeODRHash(Args));
+bool IsPartialSpecialization
+  = isa(D) ||
+  isa(D);
+Record.push_back(IsPartialSpecialization);
+  }
+}
+
 /// Get the specialization decl from an entry in the specialization list.
 template 
 typename RedeclarableTemplateDecl::SpecEntryTraits::DeclType *
@@ -205,7 +244,8 @@
 decltype(T::PartialSpecializations) (T *Common) {
   return Common->PartialSpecializations;
 }
-ArrayRef getPartialSpecializations(FunctionTemplateDecl::Common *) {
+MutableArrayRef
+getPartialSpecializations(FunctionTemplateDecl::Common *) {
   return std::nullopt;
 }
 
@@ -222,9 +262,11 @@
 assert(!Common->LazySpecializations);
   }
 
-  ArrayRef LazySpecializations;
+  using LazySpecializationInfo
+= RedeclarableTemplateDecl::LazySpecializationInfo;
+  ArrayRef LazySpecializations;
   if (auto *LS = Common->LazySpecializations)
-LazySpecializations = llvm::ArrayRef(LS + 1, LS[0]);
+LazySpecializations = llvm::ArrayRef(LS + 1, LS[0].DeclID);
 
   // Add a slot to the record for the number of specializations.
   unsigned I = Record.size();
@@ -240,12 +282,20 @@
 
   for (auto *D : Specs) {
 assert(D->isCanonicalDecl() && "non-canonical decl in set");
-AddFirstDeclFromEachModule(D, /*IncludeLocal*/true);
+AddFirstSpecializationDeclFromEachModule(D, /*IncludeLocal*/true);
+  }
+  for (auto  : LazySpecializations) {
+Record.push_back(SpecInfo.DeclID);
+Record.push_back(SpecInfo.ODRHash);
+Record.push_back(SpecInfo.IsPartial);
   }
-  Record.append(LazySpecializations.begin(), LazySpecializations.end());
 
-  // Update the 

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

Can you extend the commit log to include a description of how things are 
currently done? For example, it would be good to read about design and 
technical decisions, etc.




Comment at: clang/lib/Interpreter/CodeCompletion.cpp:79
+  if (auto Err = Interp.takeError()) {
+llvm::consumeError(std::move(Err));
+return Comps;

Why we drop the error on the floor?



Comment at: clang/lib/Interpreter/CodeCompletion.cpp:84
+  std::string AllCodeText =
+  MainInterp.getAllInput() + "\nvoid dummy(){\n" + Buffer.str() + "}";
+  auto Lines = std::count(AllCodeText.begin(), AllCodeText.end(), '\n') + 1;

I am not sure why we need to wrap this code. Can we teach the 
`TopLevelStmtDecl` to work with the code completion logic?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154382/new/

https://reviews.llvm.org/D154382

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152109: Update clang-repl documentation

[Vassil Vassilev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1ac3e13c3e4a: [clang-repl] Improve the clang-repl 
documentation. (authored by Krishna-13-cyber, committed by v.g.vassilev).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152109/new/

https://reviews.llvm.org/D152109

Files:
  clang/docs/ClangRepl.rst

Index: clang/docs/ClangRepl.rst
===
--- clang/docs/ClangRepl.rst
+++ clang/docs/ClangRepl.rst
@@ -16,22 +16,6 @@
 of Cling upstream, making them useful and available to a broader audience.
 
 
-
-Clang-Repl Usage
-
-
-
-.. code-block:: text
-
-  clang-repl> #include 
-  clang-repl> int f() { std::cout << "Hello Interpreted World!\n"; return 0; }
-  clang-repl> auto r = f();
-  // Prints Hello Interpreted World!
-
-Note that the implementation is not complete and highly experimental. We do
-not yet support statements on the global scope, for example.
-
-
 Clang-Repl Basic Data Flow
 ==
 
@@ -63,14 +47,173 @@
 
 8. The machine code is then executed.
 
+===
+Build Instructions:
+===
+
+
+.. code-block:: console
+
+   $ cd llvm-project
+   $ mkdir build
+   $ cd build
+   $ cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DLLVM_ENABLE_PROJECTS=clang -G "Unix Makefiles" ../llvm
+
+**Note here**, above RelWithDebInfo - Debug / Release
+
+.. code-block:: console
+
+   cmake --build . --target clang clang-repl -j n
+  OR
+   cmake --build . --target clang clang-repl
+
+**Clang-repl** is built under llvm-project/build/bin. Proceed into the directory **llvm-project/build/bin**
+
+.. code-block:: console
+
+   ./clang-repl
+   clang-repl>
+
+
+
+Clang-Repl Usage
+
+
+**Clang-Repl** is an interactive C++ interpreter that allows for incremental
+compilation. It supports interactive programming for C++ in a
+read-evaluate-print-loop (REPL) style. It uses Clang as a library to compile the
+high level programming language into LLVM IR. Then the LLVM IR is executed by
+the LLVM just-in-time (JIT) infrastructure.
+
+
+Basic:
+==
+
+.. code-block:: text
+
+  clang-repl> #include 
+  clang-repl> int f() { std::cout << "Hello Interpreted World!\n"; return 0; }
+  clang-repl> auto r = f();
+   // Prints Hello Interpreted World!
+
+.. code-block:: text
+
+   clang-repl> #include
+   clang-repl> using namespace std;
+   clang-repl> std::cout << "Welcome to CLANG-REPL" << std::endl;
+   Welcome to CLANG-REPL
+   // Prints Welcome to CLANG-REPL
+
+
+Function Definitions and Calls:
+===
+
+.. code-block:: text
+
+   clang-repl> #include 
+   clang-repl> int sum(int a, int b){ return a+b; };
+   clang-repl> int c = sum(9,10);
+   clang-repl> std::cout << c << std::endl;
+   19
+   clang-repl>
+
+Iterative Structures:
+=
+
+.. code-block:: text
+
+   clang-repl> #include 
+   clang-repl> for (int i = 0;i < 3;i++){ std::cout << i << std::endl;}
+   0
+   1
+   2
+   clang-repl> while(i < 7){ i++; std::cout << i << std::endl;}
+   4
+   5
+   6
+   7
+
+Classes and Structures:
+===
+
+.. code-block:: text
+
+   clang-repl> #include 
+   clang-repl> class Rectangle {int width, height; public: void set_values (int,int);\
+   clang-repl... int area() {return width*height;}};
+   clang-repl>  void Rectangle::set_values (int x, int y) { width = x;height = y;}
+   clang-repl> int main () { Rectangle rect;rect.set_values (3,4);\
+   clang-repl... std::cout << "area: " << rect.area() << std::endl;\
+   clang-repl... return 0;}
+   clang-repl> main();
+   area: 12
+   clang-repl>
+   // Note: This '\' can be used for continuation of the statements in the next line
+
+Lamdas:
+===
+
+.. code-block:: text
+
+   clang-repl> #include 
+   clang-repl> using namespace std;
+   clang-repl> auto welcome = []()  { std::cout << "Welcome to REPL" << std::endl;};
+   clang-repl> welcome();
+   Welcome to REPL
+
+Using Dynamic Library:
+==
+
+.. code-block:: text
+
+   clang-repl> %lib print.so
+   clang-repl> #include"print.hpp"
+   clang-repl> print(9);
+   9
+
+**Generation of dynamic library**
+
+.. code-block:: text
+
+   // print.cpp
+   #include 
+   #include "print.hpp"
+
+   void print(int a)
+   {
+  std::cout << a << std::endl;
+   }
+
+   // print.hpp
+   void print (int a);
+
+   // Commands
+   clang++-17  -c -o print.o print.cpp
+   clang-17 -shared print.o -o print.so
+
+Comments:
+=
+
+.. code-block:: text
+
+   clang-repl> // Comments in Clang-Repl
+   clang-repl> /* Comments in Clang-Repl */
+
+
+Closure or Termination:
+===
+
+.. code-block:: text
+
+   clang-repl>%quit
+
 
-Just like Clang, Clang-Repl can be integrated in existing applications as a
-library (via using the clangInterpreter library). This 

[PATCH] D152109: Update clang-repl documentation

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.

LGTM! If you don't have commit access I can commit this on your behalf.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152109/new/

https://reviews.llvm.org/D152109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153003: [ODRHash] Fix ODR hashing of template names

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D153003#4462388 , @ChuanqiXu wrote:

>> Oh, I guess we're somehow adding a semi-resolved form of the base class type 
>> of D into the ODR hash, which then includes details of the 
>> using-declaration. That seems wrong -- we should either (preferably) be 
>> including only the syntactic form of the base specifier (because local 
>> syntax is what the ODR covers), or the canonical type (which should be the 
>> same for both using-declarations).
>
> Got it. I'll try to fix it. Thanks for the suggestion.

Thanks @rsmith for the differential diagnosis!

@ChuanqiXu, could you add me and @Hahnfeld in the loop as that's critical for 
us.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153003/new/

https://reviews.llvm.org/D153003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152109: Update clang-repl documentation

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/docs/ClangRepl.rst:119
+   clang-repl> int c = sum(9,10);
+   clang-repl> std::cout << c;
+   19clang-repl>





Comment at: clang/docs/ClangRepl.rst:120
+   clang-repl> std::cout << c;
+   19clang-repl>
+





Comment at: clang/docs/ClangRepl.rst:145
+
+   clang-repl> #include
+   clang-repl> class Rectangle {int width, height; public: void set_values 
(int,int);int area() {return width*height;}};





Comment at: clang/docs/ClangRepl.rst:146
+   clang-repl> #include
+   clang-repl> class Rectangle {int width, height; public: void set_values 
(int,int);int area() {return width*height;}};
+   clang-repl>  void Rectangle::set_values (int x, int y) { width = x;height = 
y;}

We can use `\` to break lines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152109/new/

https://reviews.llvm.org/D152109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152109: Update clang-repl documentation

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

I think this is heading in a good direction. Please find some comments inline.




Comment at: clang/docs/ClangRepl.rst:60
+
+Clang-Repl-Usage
+





Comment at: clang/docs/ClangRepl.rst:69
+
+Build Instructions:
+===

I find it surprising to see the build instructions as part of the "usage" 
section. Can you move it out?



Comment at: clang/docs/ClangRepl.rst:88
+
+**Clang-repl** is built under llvm-project/build/bin.roceeding into the 
directory **llvm-project/build/bin**
+

A typo?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152109/new/

https://reviews.llvm.org/D152109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146809: [clang-repl] Implement Value pretty printing

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Interpreter/ValuePrinter.cpp:1
+#include "InterpreterUtils.h"
+#include "clang/AST/ASTContext.h"

Please add the llvm preamble. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146809/new/

https://reviews.llvm.org/D146809

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146809: [clang-repl] Implement Value pretty printing

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Interpreter/IncrementalParser.h:86
 
+  Parser () { return *P; }
+

Is that used?



Comment at: clang/lib/Interpreter/ValuePrinter.cpp:262
+
+static llvm::Expected CompileDecl(Interpreter ,
+   Decl *D) {

Let's add a FIXME here. The problem `CompileDecl` and `GenModule` intends to 
solve is that when we synthesize AST we need to inform the rest of the Clang 
infrastructure about it and attach the produced `llvm::Module` to the JIT so 
that it can resolve symbols from it.

In cling that is solved with a RAII object which marks a scope where the 
synthesis happens and takes care to inform the rest of the infrastructure. We 
need something similar and a little more robust maybe. Something like 
`ASTSynthesisRAII` which ultimately hides this machinery especially from the 
public API.



Comment at: clang/lib/Interpreter/ValuePrinter.cpp:442
+
+REPL_EXTERNAL_VISIBILITY std::string PrintValueRuntime(const char *const *Val) 
{
+  return PrintString(Val);

All of the `PrintValueRuntime` seem to be duplicating code. We could use the 
preprocessor to expand these to but it would require some changes which I 
believe could be done as a separate patch:

* Perhaps we should be compatible with cling here in terms of naming as that's 
a public API - there I believe we use `printValue`.
* We should try to leverage `TemplateBase.cpp::printIntegral` for integral 
types.
* We should not return `std::string` here but a `const char*` and we should 
provide somewhere storage for them. That would probably make porting to 
embedded systems easier since on many the  header is hard to support. 

I believe it would be enough to have a fixme for now.



Comment at: clang/lib/Interpreter/ValuePrinter.cpp:473
+  else if (auto *BT = DesugaredTy.getCanonicalType()->getAs()) {
+switch (BT->getKind()) {
+case BuiltinType::Bool: {

We could use the preprocessor the way we do in `Value.cpp` to expand this 
dispatcher.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146809/new/

https://reviews.llvm.org/D146809

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146809: [clang-repl] Implement Value pretty printing

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/include/clang/Interpreter/Interpreter.h:113
   llvm::Expected CompileDtorCall(CXXRecordDecl 
*CXXRD);
+  std::string CreateUniqName(std::string Base);
 

We should move this routine to its user in maybe ValuePrinter.cpp.



Comment at: clang/tools/clang-repl/CMakeLists.txt:40
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@PEBX@Z
+  
??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@P6AAEAV01@AEAV01@@Z@Z
+  
??$?6U?$char_traits@D@std@@@std@@YAAEAV?$basic_ostream@DU?$char_traits@D@std@@@0@AEAV10@D@Z

v.g.vassilev wrote:
> We should check if there is a better way to export a subset of symbols 
> through the llvm build system. Maybe @lhames, @sunho or @sgraenitz know how.
I wonder if that's related to https://reviews.llvm.org/D151620 in some way? 
That is, does D151620 fix our exports and this section becomes redundant?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146809/new/

https://reviews.llvm.org/D146809

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146809: [clang-repl] Implement Value pretty printing

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added subscribers: sunho, sgraenitz, lhames.
v.g.vassilev added a comment.

I believe this is heading in a good direction. Here are some comments from a 
partial review. Also perhaps we should extend the patch description (future 
commit message) to capture the idea of the pretty-printing including the 
dispatch, the approach how we build the AST and how to write a custom printer.




Comment at: clang/lib/Interpreter/Value.cpp:272
+
 void Value::print(llvm::raw_ostream ) const {
   assert(OpaqueType != nullptr && "Can't print default Value");

We should add some documentation for users how to write a pretty-printer for a 
custom class.

We do not seem to support the multiple inheritance case where one of the base 
classes has a pretty-printer and the other does not. See the explanation in 
Cling. It is okay to not have it at the moment but we should include a FIXME 
here saying that we do not yet support it.



Comment at: clang/lib/Interpreter/ValuePrinter.cpp:201
+  clang::Parser::ParseScope PS(
+  (), clang::Scope::FnScope | clang::Scope::BlockScope);
+

Similarly to the way we rebuild the AST during template instantiation, when we 
are synthesizing AST we do not need the lexical scope information generally. 

Removing this need for the parser here will allow us to remove 
`Interpreter::getParser` which we should not expose as an API at that stage.



Comment at: clang/lib/Interpreter/ValuePrinter.cpp:301
+
+  auto AddrOrErr = Interp.CompileDecl(WrapperFD);
+  if (!AddrOrErr)

I'd prefer to inline to body of `CompileDecl` here instead of exposing it in 
the interpreter class.



Comment at: clang/tools/clang-repl/CMakeLists.txt:40
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@PEBX@Z
+  
??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@P6AAEAV01@AEAV01@@Z@Z
+  
??$?6U?$char_traits@D@std@@@std@@YAAEAV?$basic_ostream@DU?$char_traits@D@std@@@0@AEAV10@D@Z

We should check if there is a better way to export a subset of symbols through 
the llvm build system. Maybe @lhames, @sunho or @sgraenitz know how.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146809/new/

https://reviews.llvm.org/D146809

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150937: [clang-repl] Disable all tests on unsupported platforms

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

I would support this change but instead of asking the JIT we should just add 
`-fsyntax-only` to these invocations as they do not rely on the JIT at all.




Comment at: clang/test/Interpreter/incremental-mode.cpp:3
 // RUN: clang-repl -Xcc -emit-llvm 
+// UNSUPPORTED: system-aix
 // expected-no-diagnostics

This test does not run anything, so this should be supported. Perhaps we could 
add `-fsyntax-only` mode to the RUN lines above.



Comment at: clang/unittests/Interpreter/IncrementalProcessingTest.cpp:74
+return;
   std::vector ClangArgv = {"-Xclang", "-emit-llvm-only"};
   auto CI = llvm::cantFail(IncrementalCompilerBuilder::create(ClangArgv));

Likewise, here `-fsyntax-only` mode should be enough since we do not execute 
anything.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150937/new/

https://reviews.llvm.org/D150937

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D146389#4358358 , @argentite wrote:

> Added some std::move fixes for Error -> Expected conversions
>
> We need to figure out a solution when NVPTX backend is not enabled. 
> clang-repl probably should not depends on that. Example: 
> https://lab.llvm.org/buildbot#builders/175/builds/29764

We can do `Triple::isNVPTX` and then initialize the asm printer. @lhames could 
have better idea.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146389/new/

https://reviews.llvm.org/D146389

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150139: [clang-repl] Enable basic multiline support.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev closed this revision.
v.g.vassilev added a comment.

Landed in c96c5edb58ed29e0fd936bc082ec6cae7082854e 



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150139/new/

https://reviews.llvm.org/D150139

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146389/new/

https://reviews.llvm.org/D146389

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

I believe we are at the stage of better is enemy of good now. This looks good 
to me. I'd recommend to wait for @aaron.ballman's final review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141215/new/

https://reviews.llvm.org/D141215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/include/clang/Interpreter/Value.h:23
+//
+// NOTE: Since the REPL itself could also include this runtime, extreme caution
+// should be taken when MAKING CHANGES to this file, especially when INCLUDE 
NEW

I would move the note right after the only #include we got.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141215/new/

https://reviews.llvm.org/D141215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148997: [clang] Add a new annotation token: annot_repl_input_end

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks for putting so much efforts into this!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148997/new/

https://reviews.llvm.org/D148997

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

This is looking good. Can you address my minor comments and run clang-format?




Comment at: clang/lib/Interpreter/DeviceOffload.cpp:1
+//===-- Offload.cpp - CUDA Offloading ---*- C++ 
-*-===//
+//

v.g.vassilev wrote:
> Likewise.
ping.



Comment at: clang/lib/Interpreter/DeviceOffload.h:1
+//===-- Offload.h - Device Offloading ---*- C++ 
-*-===//
+//

We should rename `Offload.h` to `DeviceOffload.h`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146389/new/

https://reviews.llvm.org/D146389

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/CodeGen/ModuleBuilder.cpp:39
 const PreprocessorOptions  // Only used for debug info.
-const CodeGenOptions CodeGenOpts;  // Intentionally copied in.
+CodeGenOptions CodeGenOpts;  // Intentionally copied in.
 

argentite wrote:
> v.g.vassilev wrote:
> > IIUC history correctly, here the intentional copy was to prevent some 
> > layering violation for what was called in 2009 `CompileOpts`. I believe 
> > that is not the case, can you check if we can take a const reference here? 
> I don't understand how the reference causes layering violation but if I 
> change it to a const reference instead, the option modification code becomes 
> slightly less awkward and all tests seem to be fine.
Let's try that then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146389/new/

https://reviews.llvm.org/D146389

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/CodeGen/ModuleBuilder.cpp:39
 const PreprocessorOptions  // Only used for debug info.
-const CodeGenOptions CodeGenOpts;  // Intentionally copied in.
+CodeGenOptions CodeGenOpts;  // Intentionally copied in.
 

IIUC history correctly, here the intentional copy was to prevent some layering 
violation for what was called in 2009 `CompileOpts`. I believe that is not the 
case, can you check if we can take a const reference here? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146389/new/

https://reviews.llvm.org/D146389

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits