Skip to content

Commit

Permalink
Print file path in detailed errors when jumping files (chapel-lang#23520
Browse files Browse the repository at this point in the history
)

This aims to resolve chapel-lang#21656.

Though I've read Brad's opinion on the various sketches, I am not
inclined to put the filename on the same line as the previous error
message. The reason for this is that it doesn't mesh well with the
current approach we take (each piece of the error message is a distinct
call that writes text to a stream). To be able to splice the file from
code output back into the line before it, we'd need the code that prints
the error message to be coupled with the code that prints the lines of
Chapel. Furthermore, the approach is not resilient to non-standard
phrasing of the message above (it modifies them grammatically!).

Thus, I've taken the approach of Rust error messages, and print the
filename with an `-->` character. I think using delimiters makes the
filename less likely to blend in with the sentence above (and, like I
described above, actually making it blend in properly is hard).

The error message from the issue now looks as follows:

<img width="368" alt="Screen Shot 2023-09-21 at 5 08 18 PM"
src="https://github.com/chapel-lang/chapel/assets/4361282/4ba23642-0453-4807-b928-43916ca4c569">

Reviewed by @mppf -- thanks!

## Testing
- [x] paratest
  • Loading branch information
DanilaFe authored Sep 28, 2023
2 parents c155095 + c877d11 commit 4fab72c
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 8 deletions.
9 changes: 9 additions & 0 deletions frontend/include/chpl/framework/ErrorWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,15 @@ class ErrorWriter : public ErrorWriterBase {
std::ostream& oss_;
ErrorWriterBase::OutputFormat outputFormat_;
bool useColor_;
std::string lastFilePath_;

/* Called when the error tries to print a particular file path to indicate
the location of an error. Returns true if this is needed, which happens
when the previously-printed file path was different. Otherwise,
the error message can skip printing file path information, since
it has not changed.
*/
bool noteFilePath(std::string newPath);

void setColor(TermColorName color);

Expand Down
47 changes: 39 additions & 8 deletions frontend/lib/framework/ErrorWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ static std::string fileText(Context* context, const Location& loc) {
return fileText.text();
}

bool ErrorWriter::noteFilePath(std::string newPath) {
bool toReturn = lastFilePath_ != newPath;
lastFilePath_ = std::move(newPath);
return toReturn;
}

void ErrorWriter::setColor(TermColorName color) {
if (useColor_) {
oss_ << getColorFormat(color);
Expand All @@ -104,18 +110,29 @@ static TermColorName kindColor(ErrorBase::Kind kind) {
return CLEAR;
}

static void writeFile(Context* context,
std::ostream& oss,
const Location& loc) {
static std::string locToPath(Context* context, const Location& loc) {
UniqueString pathUstr = loc.path();
if (context) pathUstr = context->adjustPathForErrorMsg(pathUstr);
auto path = pathUstr.c_str();
int lineno = loc.line();
bool validPath = (path != nullptr && path[0] != '\0');
if (validPath) return path;
return "";
}

static void writeFile(Context* context,
std::ostream& oss,
const Location& loc,
std::string* outFilePath = nullptr) {
int lineno = loc.line();
auto path = locToPath(context, loc);
if (outFilePath) *outFilePath = path;

if (validPath && lineno > 0) oss << path << ":" << lineno;
else if (validPath) oss << path;
else oss << "(unknown location)";
if (!path.empty()) {
if (lineno > 0) oss << path << ":" << lineno;
else oss << path;
} else {
oss << "(unknown location)";
}
}

void ErrorWriter::writeHeading(ErrorBase::Kind kind, ErrorType type,
Expand All @@ -129,7 +146,13 @@ void ErrorWriter::writeHeading(ErrorBase::Kind kind, ErrorType type,
oss_ << kindText(kind);
setColor(CLEAR);
oss_ << " in ";
writeFile(context, oss_, errordetail::locate(context, loc));

// Printing the header prints the file path, so we need to update the
// 'lastFilePath_' field.
std::string printedPath;
writeFile(context, oss_, errordetail::locate(context, loc), &printedPath);
noteFilePath(std::move(printedPath));

if (outputFormat_ == DETAILED) {
// Second part of the error decoration
const char* name = ErrorBase::getTypeName(type);
Expand Down Expand Up @@ -207,6 +230,14 @@ void ErrorWriter::writeCode(const Location& location,
size_t codeIndent = gutterSize+3;
int lineNumber = location.firstLine();

// Print the file path if it's changed since the last code block. Printing
// a code block will display the file path if needed, so lastFilePath_ needs
// to be updated.
if (noteFilePath(locToPath(context, location))) {
printBlank(oss_, codeIndent - 1);
oss_ << "--> " << lastFilePath_ << std::endl;
}

printBlank(oss_, codeIndent);
oss_ << "|";
oss_ << std::endl;
Expand Down

0 comments on commit 4fab72c

Please sign in to comment.