Page MenuHomePhabricator

[NewPM] teach -passes= to emit meaningful error messages

Authored by fedor.sergeev on Oct 13 2018, 10:07 AM.



All the PassBuilder::parse interfaces now return descriptive StringError
instead of a plain bool. It allows to make -passes/aa-pipeline parsing
errors context-specific and thus less confusing.

TODO: ideally we should also make suggestions for misspelled pass names,
but that requires some extensions to PassBuilder.

Diff Detail


Event Timeline

fedor.sergeev created this revision.Oct 13 2018, 10:07 AM
chandlerc accepted this revision.Oct 15 2018, 3:40 AM

LGTM, this is a great improvement IMO!

This revision is now accepted and ready to land.Oct 15 2018, 3:40 AM
philip.pfaffe accepted this revision.Oct 15 2018, 4:14 AM

This looks good to me, minor nits inline.

149 ↗(On Diff #169569)

This comment is misleading now.

139 ↗(On Diff #169569)

Here and below: Err is unused. Do you want to do anything with it? If you don't then a) you don't need the name and b) a pipeline will cause an unchecked error abort, with an error message that will look weird. So if you don't care about the error at least consume it. I'd prefer printing a nicer message though.

adding error handling for repeated EP pipeline parsing.

fixing return code for -passes-ep* tests (should not be failing).

This revision was automatically updated to reflect the committed changes.