Page MenuHomePhabricator

FileCheck: Return parse error w/ Error & Expected
ClosedPublic

Authored by thopre on Jun 11 2019, 1:38 AM.

Details

Summary

Make use of Error and Expected to bubble up diagnostics and force
checking of errors in the callers.

Event Timeline

thopre created this revision.Jun 11 2019, 1:38 AM

Looks good to me. Just two minor comments:

This adds quite a lot of return make_error<ParseErrorInfo>(SM.GetMessage(SMLoc::getFromPointer(Foo.data()), SourceMgr::DK_Error, "...")));
Maybe adding a helper would make sense? Something like:

Error ParseErrorInfo::get(const SourceManager& SM, const char* LocPtr, const Twine& ErrMsg) {
   return make_error<ParseErrorInfo>(SM.GetMessage(SMLoc::getFromPointer(LocPtr), SourceMgr::DK_Error,  ErrMsg)));
}

Then the above can just be return ParseErrorInfo::get(SM, Foo.data(), "...");

thopre updated this revision to Diff 204238.Jun 12 2019, 2:13 AM
  • Fix bugs in unit test when running with Error checks enabled
  • Make eval() functions return an Expected instead of Optional
  • Create helper functions to create ParseError and its associated diagnostic
  • Rename ParseErrorInfo into FileCheckParseError
thopre marked 3 inline comments as done.Jun 17 2019, 2:29 PM
thopre added inline comments.
llvm/include/llvm/Support/FileCheck.h
351–352

This can be simplified to Diagnostic.print(nullptr, OS);

llvm/lib/Support/FileCheck.cpp
617–620

Perhaps worth making match() return an error via an Expected<size_t> return value.

thopre updated this revision to Diff 205358.Jun 18 2019, 8:06 AM
  • Convert match to return Expected as well
  • Simplify FileCheckErrorDiagnostic logging
thopre updated this revision to Diff 205361.Jun 18 2019, 8:29 AM

Fix unit test

thopre updated this revision to Diff 205435.Jun 18 2019, 2:29 PM

Replace assert(false) by llvm_unreachable

thopre updated this revision to Diff 205441.Jun 18 2019, 2:54 PM

Fix code style issues

thopre updated this revision to Diff 205529.Jun 19 2019, 2:15 AM

Remove "represent value" -> "store value" change

This revision is now accepted and ready to land.Jun 19 2019, 7:09 AM
thopre updated this revision to Diff 205687.Jun 19 2019, 3:31 PM
  • Remove useless llvm_unreachable from handleAllErrors
  • Move comment to apply to both ignored errors
This revision was automatically updated to reflect the committed changes.