The LLVM Style Guide says the following about error and warning messages [1]: > [T]o match error message styles commonly produced by other tools, > start the first sentence with a lowercase letter, and finish the last > sentence without a period, if it would end in one otherwise. I often provide this feedback during code review, but we still have a bunch of places where we have inconsistent error message, which bothers me as a user. This PR identifies a handful of those places and updates the messages to be consistent. [1] https://llvm.org/docs/CodingStandards.html#error-and-warning-messages
Given this simple test case:
```
// foo.h
extern int* ptr;
inline void g(int x) {
*ptr = x; // should raise a SIGILL
}
//--------------
// foo.cc
#include "foo.h"
int* ptr;
//--------------
// main.cc
#include "foo.h"
int main() {
g(123); // Call the inlined function and crash
return 0;
}
$ clang -g main.cc foo.cc -o main.out
$ lldb main.out
```
When you run `main.out` under lldb, it'd stop inside `void g(int)`
because of the crash.
The stack trace would show that it had crashed in `foo.h`, but if you do
`list foo.h:2`, lldb would complain that it could not find the source
file, which is confusing.
This patch make `list` work with headers.
While looking at how to make Function::GetEndLineSourceInfo (which is only used in "command source") work with discontinuous functions, I realized there are other corner cases that this function doesn't handle. The code assumed that the last line entry in the function will also correspond to the last source line. This is probably true for unoptimized code, but I don't think we can rely on the optimizer to preserve this property. What's worse, the code didn't check that the last line entry belonged to the same file as the first one, so if this line entry was the result of inlining, we could end up using a line from a completely different file. To fix this, I change the algorithm to iterate over all line entries in the function (which belong to the same file) and find the max line number out of those. This way we can naturally handle the discontinuous case as well. This implementation is going to be slower than the previous one, but I don't think that matters, because: - this command is only used rarely, and interactively - we have plenty of other code which iterates through the line table I added some basic tests for the function operation. I don't claim the tests to be comprehensive, or that the function handles all edge cases, but test framework created here could be used for testing other fixes/edge cases as well.
Lots of code around LLDB was directly accessing the target's section load list. This NFC patch makes the section load list private so the Target class can access it, but everyone else now uses accessor functions. This allows us to control the resolving of addresses and will allow for functionality in LLDB which can lazily resolve addresses in JIT plug-ins with a future patch.
When retrieving the location of the function declaration, we were dropping the file component on the floor, which resulted in an amusingly confusing situation were we displayed the file containing the implementation of the function, but used the line number of the declaration. This patch fixes that. It required a small refactor Function::GetStartLineSourceLineInfo to return a SupportFile (instead of just the file spec), which in turn necessitated changes in a couple of other places as well.
Now that more parts of LLDB know about SupportFiles, avoid going through FileSpec (and losing the Checksum in the process). Instead, use the SupportFile directly.
To support detecting MD5 checksum mismatches, deal with SupportFiles rather than a plain FileSpecs in the SourceManager.
To support detecting MD5 checksum mismatches, store a SupportFile rather than a plain FileSpec in SourceManager::File.
This patch removes all of the Set.* methods from Status. This cleanup is part of a series of patches that make it harder use the anti-pattern of keeping a long-lives Status object around and updating it while dropping any errors it contains on the floor. This patch is largely NFC, the more interesting next steps this enables is to: 1. remove Status.Clear() 2. assert that Status::operator=() never overwrites an error 3. remove Status::operator=() Note that step (2) will bring 90% of the benefits for users, and step (3) will dramatically clean up the error handling code in various places. In the end my goal is to convert all APIs that are of the form ` ResultTy DoFoo(Status& error) ` to ` llvm::Expected<ResultTy> DoFoo() ` How to read this patch? The interesting changes are in Status.h and Status.cpp, all other changes are mostly ` perl -pi -e 's/\.SetErrorString/ = Status::FromErrorString/g' $(git grep -l SetErrorString lldb/source) ` plus the occasional manual cleanup.