labath added a comment.

In D55384#1321970 <https://reviews.llvm.org/D55384#1321970>, @zturner wrote:

> Clang-cl emits `S_LOCAL` symbols while MSVC emits `S_REGREL32` and 
> `S_REGISTER` symbols.  So, to get more test coverage, I added an MSVC test as 
> well.


I don't want to block this patch, but my thoughts after reading this is that 
this is repeating the same mistakes that we did with dwarf. The reason we need 
so much to run our tests in so many configurations is that we haven't found a 
way way to handle the fact that some things can be expressed in multiple ways 
in the debug info other than compiling the source code with the compiler which 
happens to use that dialect. Then, if a later e.g. clang chooses to do the same 
thing that msvc does, we lose coverage here without anyone noticing. For 
low-level details like these, I believe a test where the type of the symbol is 
explicit would be more appropriate (for dwarf that might be a .s file, I don't 
know if that would work for pdbs).


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

https://reviews.llvm.org/D55384



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

Reply via email to