Skip to content

Commit

Permalink
Rename Program::Load to Program::load (#384)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #384

This is a commonly-used method that violates our style guide.
ghstack-source-id: 200971697
exported-using-ghexport

Reviewed By: cccclai

Differential Revision: D49345074

fbshipit-source-id: a6f13106e59d60d75a8e1452e9ba790b1886bd63
  • Loading branch information
dbort authored and facebook-github-bot committed Sep 18, 2023
1 parent 3acbbb1 commit 8a5f3e8
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 12 deletions.
4 changes: 2 additions & 2 deletions runtime/executor/program.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ Result<executorch_flatbuffer::ExecutionPlan*> get_execution_plan(

} // namespace

/* static */ Result<Program> Program::Load(
/* static */ Result<Program> Program::load(
DataLoader* loader,
Program::Verification verification) {
EXECUTORCH_SCOPE_PROF("Program::Load");
EXECUTORCH_SCOPE_PROF("Program::load");

// See if the program size is in the header.
size_t program_size = 0;
Expand Down
9 changes: 8 additions & 1 deletion runtime/executor/program.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,17 @@ class Program final {
* @param[in] verification The type of verification to do before returning
* success.
*/
__ET_NODISCARD static Result<Program> Load(
__ET_NODISCARD static Result<Program> load(
DataLoader* loader,
Verification verification = Verification::Minimal);

/// DEPRECATED: Use the lowercase `load()` instead.
__ET_DEPRECATED __ET_NODISCARD static Result<Program> Load(
DataLoader* loader,
Verification verification = Verification::Minimal) {
return load(loader, verification);
}

// Movable, to be compatible with Result.
Program(Program&&) noexcept = default;
~Program() = default;
Expand Down
25 changes: 16 additions & 9 deletions runtime/executor/test/program_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,15 @@ using torch::executor::testing::ProgramTestFriend;
TEST_F(ProgramTest, DataParsesWithMinimalVerification) {
// Parse the Program from the data.
Result<Program> program =
Program::Load(add_loader_.get(), Program::Verification::Minimal);
Program::load(add_loader_.get(), Program::Verification::Minimal);

// Should have succeeded.
EXPECT_EQ(program.error(), Error::Ok);
}

TEST_F(ProgramTest, DataParsesWithInternalConsistencyVerification) {
// Parse the Program from the data.
Result<Program> program = Program::Load(
Result<Program> program = Program::load(
add_loader_.get(), Program::Verification::InternalConsistency);

// Should have succeeded.
Expand Down Expand Up @@ -139,7 +139,7 @@ TEST_F(ProgramTest, BadMagicFailsToLoad) {
// Parse the Program from the data. Use minimal verification to show that
// even this catches the header problem.
Result<Program> program =
Program::Load(&data_loader, Program::Verification::Minimal);
Program::load(&data_loader, Program::Verification::Minimal);

// Should fail.
ASSERT_EQ(program.error(), Error::InvalidProgram);
Expand All @@ -152,7 +152,7 @@ TEST_F(ProgramTest, BadMagicFailsToLoad) {
{
// Parse the Program from the data again.
Result<Program> program =
Program::Load(&data_loader, Program::Verification::Minimal);
Program::load(&data_loader, Program::Verification::Minimal);

// Should now succeed.
ASSERT_EQ(program.error(), Error::Ok);
Expand All @@ -170,7 +170,7 @@ TEST_F(ProgramTest, VerificationCatchesTruncation) {
BufferDataLoader half_data_loader(full_data->data(), full_data_len / 2);

// Loading with full verification should fail.
Result<Program> program = Program::Load(
Result<Program> program = Program::load(
&half_data_loader, Program::Verification::InternalConsistency);
ASSERT_EQ(program.error(), Error::InvalidProgram);
}
Expand All @@ -195,7 +195,7 @@ TEST_F(ProgramTest, VerificationCatchesCorruption) {

// Should fail to parse corrupted data when using full verification.
Result<Program> program =
Program::Load(&data_loader, Program::Verification::InternalConsistency);
Program::load(&data_loader, Program::Verification::InternalConsistency);
ASSERT_EQ(program.error(), Error::InvalidProgram);
}

Expand All @@ -216,14 +216,14 @@ TEST_F(ProgramTest, UnalignedProgramDataFails) {

// Should refuse to accept unaligned data.
Result<Program> program =
Program::Load(&data_loader, Program::Verification::Minimal);
Program::load(&data_loader, Program::Verification::Minimal);
ASSERT_NE(program.error(), Error::Ok);
}

TEST_F(ProgramTest, LoadSegmentWithNoSegments) {
// Load a program with no segments.
Result<Program> program =
Program::Load(add_loader_.get(), kDefaultVerification);
Program::load(add_loader_.get(), kDefaultVerification);
EXPECT_EQ(program.error(), Error::Ok);

// Loading a segment should fail.
Expand Down Expand Up @@ -305,7 +305,7 @@ TEST_F(ProgramTest, HeaderNotPresent) {
TEST_F(ProgramTest, getMethods) {
// Parse the Program from the data.
Result<Program> program_res =
Program::Load(multi_loader_.get(), kDefaultVerification);
Program::load(multi_loader_.get(), kDefaultVerification);
EXPECT_EQ(program_res.error(), Error::Ok);

Program program(std::move(program_res.get()));
Expand All @@ -319,3 +319,10 @@ TEST_F(ProgramTest, getMethods) {
EXPECT_TRUE(res2.ok());
EXPECT_EQ(strcmp(res2.get(), "forward2"), 0);
}

// Test that the deprecated Load method (capital 'L') still works.
TEST_F(ProgramTest, DEPRECATEDLoad) {
// Parse the Program from the data.
Result<Program> program_res = Program::Load(multi_loader_.get());
EXPECT_EQ(program_res.error(), Error::Ok);
}

0 comments on commit 8a5f3e8

Please sign in to comment.