dblaikie added inline comments.
================
Comment at: lldb/source/Symbol/TypeSystem.cpp:331
"TypeSystem for language " +
- llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+ llvm::StringRef(Language::GetNameForLanguageType(language)) +
" doesn't exist",
----------------
teemperor wrote:
> dblaikie wrote:
> > Perhaps Language::GetNameForLanguageType should return StringRef to start
> > with?
> >
> > (& guessing there should be an lldb test case demonstrating this fix?)
> The problem is that this is used in LLDB's SB API which returns a const char*
> (and we can't change that). So we can't return a StringRef as that doesn't
> convert back to const char * (and we can't go over std::string or anything
> like that as the API doesn't transfer ownership back to the caller). I added
> an alternative method to save some typing (and maybe we can go fully
> StringRef in the future if we ever change the SB API strings).
>
> And we don't test log message content in LLDB. Also the log message only
> happens on systems with unsupported type system (like MIPS or something like
> that). And we don't have any such bot that would even run the test.
Those both seem a bit problematic...
Is there no API migration strategy in the SB API? Introducing new versions of
functions and deprecating old ones?
And testing - clearly this code was buggy and worth fixing, so it ought to be
worth testing. Any chance of unit testing/API-level testing any of this?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65942/new/
https://reviews.llvm.org/D65942
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
