revert duplicate filename changes and test#301
Open
HimanshuChoudhary-Xilinx wants to merge 1 commit into
Open
Conversation
Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR appears to revert/disable recently added “duplicate filename” validation and the associated negative tests across the C++ assembler/preprocessor and CMake-based test suites.
Changes:
- Commented out duplicate
.include/ filename “seen” checks in the ASM preprocessor (asm_parser). - Commented out duplicate virtual-file (
file_artifact::add_vfile) validation in the assembler. - Disabled (commented out) the negative tests that exercised these error paths.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
test/cpp_test/cpp_api/CMakeLists.txt |
Disables building and registering the duplicate-filename negative test binary. |
test/aie4-ctrlcode/CMakeLists.txt |
Disables the duplicate_include test subdirectory. |
src/cpp/preprocessor/asm/asm_parser.h |
Comments out is_filename_seen helpers used for include duplicate/cycle detection. |
src/cpp/preprocessor/asm/asm_parser.cpp |
Comments out the .include duplicate/cycle guard. |
src/cpp/assembler/aiebu_assembler.cpp |
Comments out duplicate-name checks in file_artifact_impl::add_vfile(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1514
to
+1516
| //if (m_parserptr->is_filename_seen(filename)) | ||
| // throw error(error::error_code::invalid_input, | ||
| // "duplicate asm file name \"" + filename + "\""); |
Comment on lines
+253
to
+255
| //bool is_filename_seen(const std::string& fname) const { | ||
| // return m_index.find(fname) != m_index.end(); | ||
| //} |
Comment on lines
+836
to
+838
| //bool is_filename_seen(const std::string& fname) const { | ||
| // return m_filename_table.is_filename_seen(fname); | ||
| //} |
Comment on lines
451
to
455
| { | ||
| if (m_artifacts.count(name)) | ||
| throw error(error::error_code::invalid_input, | ||
| "duplicate asm file name \"" + name + "\" added in artifacts"); | ||
| //if (m_artifacts.count(name)) | ||
| // throw error(error::error_code::invalid_input, | ||
| // "duplicate asm file name \"" + name + "\" added in artifacts"); | ||
| m_artifacts[name] = buffer; |
Comment on lines
459
to
463
| { | ||
| if (m_artifacts.count(name)) | ||
| throw error(error::error_code::invalid_input, | ||
| "duplicate asm file name \"" + name + "\" added in artifacts"); | ||
| //if (m_artifacts.count(name)) | ||
| // throw error(error::error_code::invalid_input, | ||
| // "duplicate asm file name \"" + name + "\" added in artifacts"); | ||
| m_artifacts[name] = std::move(buffer); |
Comment on lines
+31
to
+37
| #set(AIEBU_ERROR_TESTNAME "aiebu_error_cpp") | ||
| #add_executable(${AIEBU_ERROR_TESTNAME} aiebu_error_test.cpp) | ||
| #target_link_libraries(${AIEBU_ERROR_TESTNAME} PRIVATE aiebu_static) | ||
| #target_include_directories(${AIEBU_ERROR_TESTNAME} PRIVATE ${AIEBU_SOURCE_DIR}/src/cpp/include) | ||
| #if (NOT WIN32) | ||
| # target_link_libraries(${AIEBU_ERROR_TESTNAME} PRIVATE pthread) | ||
| #endif() |
Comment on lines
+119
to
+121
| #add_test(NAME "aiebu_error_duplicate_asm_filename" | ||
| # COMMAND ${AIEBU_ERROR_TESTNAME} | ||
| # WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) |
| add_subdirectory(multi_graph) | ||
| add_subdirectory(opcode_checks) | ||
| add_subdirectory(duplicate_include) | ||
| #add_subdirectory(duplicate_include) |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem solved by the commit
Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
How problem was solved, alternative solutions (if any) and why they were rejected
Risks (if any) associated the changes in the commit
What has been tested and how, request additional testing if necessary
Documentation impact (if any)