Adds stream interface for parseLibertyFile#350
Conversation
Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
| #pragma once | ||
|
|
||
| #include <functional> | ||
| #include <istream> |
There was a problem hiding this comment.
<istream> is fine but <iosfwd> would suffice in LibertyParser.hh since only a reference is declared, slightly reducing transitive include cost across the many translation units that pull in this header
| LibertyGroupVisitor *library_visitor, | ||
| Report *report) | ||
| { | ||
|
|
There was a problem hiding this comment.
Please remove blank line here.
| else | ||
| throw FileNotReadable(filename); | ||
| } | ||
|
|
There was a problem hiding this comment.
Please add a comment here "// Parse Liberty from an in-memory stream.
// filename is used only as a label in diagnostic messages;
// it need not correspond to a file on disk."
It was a bit confusing to me how file_contents and filename interacted- but once I got it, I think it makes sense to add a one-line explanation.
|
It would be nice to have a stream based unit testcase. Here is something you can add as a C++ unit test to liberty/test/cpp/TestLibertyStaCallbacks.cc. It mirrors the existing ParserSaveAll test but exercises the new stream overload, and crucially it uses no temp file, which is the point of the new API. You'll also need this include at the top of the file (next to the existing
|
This allows users of the STA api to read liberty from an in memory copy.