[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D78972#2011571 <https://reviews.llvm.org/D78972#2011571>, @jingham wrote:

> That one does work all the way, including calling the function.  That should be surprising.   It shouldn't with an unpatched lldb, right?


Yep, I realized that is not the best example after posting that comment.

> We do this whole dance again every time we run this expression.  It looks like we don't look for instances of a template if we know how to make it, we just make it again.  That actually makes sense.  When you are compiling, you don't know if you will find an instantiation, so you have to make it.  Then you leave it to the linker to remove duplicates.

You're describing the "normal" `linkonce` semantics. Things get more complex once you start having explicit instantiations: `template void f<int>();` forces a compiler to emit an instantiation even though it is not needed (this results in the `weak_odr` linkage. `extern template void f<int>();` inhibits instantiation -- the compiler will assume you have an explicit instantiation elsewhere. The fact that the explicit instantiation is "weak" allows for some interesting effects, but it's tricky to demonstrate them, as they all involve UB. Once can see some of that with normal compilation like this:

  $ head a.cc b.cc c.cc
  ==> a.cc <==
  #include <cstdio>
  
  template<typename T> void f() { printf("Hello T\n"); }
  
  template void f<int>(); // weak
  
  ==> b.cc <==
  #include <cstdio>
  
  extern "C" void _Z1fIiEvv() { printf("Hello _Z1fIiEvv\n"); } // strong
  
  ==> c.cc <==
  #include <cstdio>
  
  template<typename T> void f() { printf("Hello T\n"); }
  
  extern template void f<int>(); // inhibit instantiation
  
  int main() {
    f<int>();
  }
  $ c++ a.cc c.cc
  $ ./a.out 
  Hello T
  $ c++ a.cc c.cc b.cc
  $ ./a.out 
  Hello _Z1fIiEvv

I don't know whether any of this is relevant for top-level expressions.

Incidentally it looks like we have a problem with function template specializations in top-level expressions, though I don't think that is related to this patch in any way:

  (lldb) expr --top-level --
  Enter expressions, then terminate with an empty line to evaluate:
    1: template<typename T> void f() { (void) printf("Hello T\n"); } 
    2: template<> void f<int>() { (void) printf("Hello int\n"); } 
    3: void g() { f<int>(); } 
  (lldb) p g()
  Hello int
  (lldb) p f<int>()
  Hello T



> Note that this is all about creating new code in a running program, and it almost seems like you really should use the new version you created if it is any different.  Or anyway, it's not entirely clear what the rule should be.  And anyway, the thing we have to do is not NOT export these symbols, it is really "figure out if they exist somewhere else and are equivalent" and use those, otherwise export these.  Since I don't yet have an example where there are two versions that would get used, I don't want to try to write code to do that blind.

Yep, at this point I am pretty lost about what should be the correct behavior, and whether the change is detectable with c++. However, I don't like the fact that the change has no test coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78972