v.g.vassilev added a comment.

Thanks for working on this, looks like we are heading in a good direction!



================
Comment at: clang/test/Interpreter/dynamic-library.cpp:1
+// RUN: head -n 7 %s | %clang -xc++ -o %T/libdynamic-library-test.so -fPIC 
-shared -
+int ultimate_answer = 0;
----------------
sgraenitz wrote:
> The use of `head` and `tail` here is a creative solution, but I wonder how 
> well it scales for similar cases in the future in case this becomes a role 
> model. We have this situation a lot in LLDB: compile an input file and use 
> the output in a RUN line. It typically uses separate input files for such 
> purposes (note that "Inputs" folders must be excluded from test discovery 
> somewhere in lit config), e.g.:
> https://github.com/llvm/llvm-project/blob/release/16.x/lldb/test/Shell/Breakpoint/jit-loader_jitlink_elf.test
> 
> What do you think?
I agree with that. I've seen that some clang tests surround the code in 
question with `#ifdef LIB` and then in the invocation they add `-DLIB`.


================
Comment at: clang/tools/clang-repl/ClangRepl.cpp:126
       }
+      if (Line->rfind(R"(%lib )", 0) == 0) {
+        if (auto Err = Interp->LoadDynamicLibrary(Line->data() + 5)) {
----------------
argentite wrote:
> sgraenitz wrote:
> > I see this is analog to existing code, but is there a good reason for using 
> > the multi-line string syntax here?
> Yeah I am confused as well. But I tried to maintain the style.
Independently, I think these commands should be available via pragma directives 
as well. Maybe in a separate revision we should create something like `pragma 
clang-repl/clang-interpreter load library name`. That would allow people to use 
that in their "scripts" which can be quite useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141824

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

Reply via email to