Skip to content

Adds stream interface for parseLibertyFile#350

Open
QuantamHD wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
QuantamHD:stream_liberty
Open

Adds stream interface for parseLibertyFile#350
QuantamHD wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
QuantamHD:stream_liberty

Conversation

@QuantamHD
Copy link
Copy Markdown
Contributor

@QuantamHD QuantamHD commented Apr 20, 2026

This allows users of the STA api to read liberty from an in memory copy.

Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 20, 2026

CLA assistant check
All committers have signed the CLA.

Comment thread liberty/LibertyParser.hh
#pragma once

#include <functional>
#include <istream>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<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

Comment thread liberty/LibertyParser.cc
LibertyGroupVisitor *library_visitor,
Report *report)
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove blank line here.

Comment thread liberty/LibertyParser.cc
else
throw FileNotReadable(filename);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@dsengupta0628
Copy link
Copy Markdown
Contributor

dsengupta0628 commented May 1, 2026

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.

// Verifies in-memory parsing via std::istream& without touching the filesystem.
  TEST_F(StaLibertyTest, ParserStreamOverload) {
    const char *content = R"(
  library(test_stream_overload) {
    delay_model : table_lookup ;
    time_unit : "1ns" ;
    voltage_unit : "1V" ;
    current_unit : "1mA" ;
    capacitive_load_unit(1, ff) ;
    define(custom_attr, cell, float) ;
    my_variable = 42.0 ;
    cell(SV1) {
      area : 1.0 ;
      pin(A) { direction : input ; capacitance : 0.01 ; }
      pin(Z) { direction : output ; function : "A" ; }
    }
  }
  )";

    // Feed the parser directly from memory — no temp file involved.
    std::stringstream stream(content);
    RecordingLibertyVisitor visitor;
    parseLibertyFile(stream, "<in-memory>", &visitor, sta_->report());

    // Same expectations as the file-path variant: the visitor saw a library
    // group with one define, one variable, and a cell with area=1.0.
    EXPECT_GT(visitor.begin_count, 0);
    EXPECT_EQ(visitor.begin_count, visitor.end_count);
    ASSERT_EQ(visitor.root_groups.size(), 1u);
    const LibertyGroup *library = visitor.root_groups.front();
    ASSERT_NE(library, nullptr);
    EXPECT_EQ(library->defineMap().size(), 1u);
    EXPECT_EQ(visitor.variables.size(), 1u);
    EXPECT_GT(visitor.simple_attrs.size(), 0u);

    const LibertyGroup *cell = library->findSubgroup("cell");
    ASSERT_NE(cell, nullptr);
    float area = 0.0f;
    bool exists = false;
    cell->findAttrFloat("area", area, exists);
    EXPECT_TRUE(exists);
    EXPECT_FLOAT_EQ(area, 1.0f);
  }

You'll also need this include at the top of the file (next to the existing <string>):

#include <sstream>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants