fix: resolve latest codecheck findings in ptoas and ptobc - #1474
Draft
jimmychou0 wants to merge 2 commits into
Draft
fix: resolve latest codecheck findings in ptoas and ptobc#1474jimmychou0 wants to merge 2 commits into
jimmychou0 wants to merge 2 commits into
Conversation
- const-ify the five pure Encoder::encode*Immediate helpers that do not
touch mutable Encoder state (encodeConstantImmediate still interns
into file and stays non-const)
- drop redundant includes: ptoas.cpp (cctype/csignal/cstdlib/cstring/
sys/types.h/unistd.h), ptoas_pipeline.cpp (csignal/cstring/unistd.h),
ptoas_cpp_rewrite.cpp and ptoas_name_hints.cpp (Arith/Tensor
BufferizableOpInterfaceImpl)
- replace unsigned += bool counters in VFSIMTSizePatcher with explicit
if/increment blocks
- make if (info.has_variant_u8) an explicit != 0 comparison and cast the
int hex-decode operand to unsigned before the bitwise or
- own the decoded mlir::Block with std::unique_ptr until region
push_back so a decode exception can no longer leak it (SecK finding)
- name the hex-escape length/offset and SmallString capacity constants
instead of magic 3/2/128
- remove the stray blank line inside parseNameHintMarker, the orphan //
line inside the driver job-topology diagram, and the run of blank
lines after namespace { in ptoas.cpp
Validated on the 144 host (LLVM 19.1.7 assert build, GCC 13.3): full
build warning-free under -Werror and CTest 69/69 passes.
The ptoas.cpp split copied the same ~70-line include list into all four translation units, which the duplication checker now flags across ptoas.cpp / ptoas_cpp_rewrite.cpp / ptoas_name_hints.cpp / ptoas_pipeline.cpp. Move the include set common to all four TUs into the existing internal header ptoas_internal.h and keep only the TU-specific includes in each .cpp (Arith/Tensor Bufferizable registrations in ptoas.cpp and ptoas_pipeline.cpp, <cctype> in the two text-rewrite TUs), dropping includes already removed as unused. Net -232 duplicated lines; each TU compiles with an unchanged effective include set. Validated on the 144 host (LLVM 19.1.7 assert build): warning-free build under -Werror and CTest 69/69.
jimmychou0
force-pushed
the
zjm/codecheck-fix-4-rebased
branch
from
September 7, 2026 01:20
f605624 to
935eabc
Compare
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.
Summary
Resolve the latest codecheck scan for
tools/ptoasandtools/ptobc(41 findings: 36 fixed, 5 not-applicable with reasons) and fix the CI duplication findings across the split ptoas TUs. Rebased cleanly onto the currentmain(75e4a22) — replaces the closed #1462; base moved frommaintomaster(new default branch).Encoder::encode*Immediatehelpers (CmpPredicate/Event/MakeTensorView/PartitionView/AllocTile);encodeConstantImmediatestill mutatesfileviainternTypeand stays non-const.if (info.has_variant_u8)→!= 0;unsigned += boolcounters → explicitif { ++n; }; int hex operand explicitly cast to unsigned before|.mlir::Blockis owned bystd::unique_ptruntilregion.push_back, so a mid-decode exception can no longer leak it.SmallStringcapacity (128) constants.parseNameHintMarker, the orphan//in the driver job-topology diagram, and the run of blank lines afternamespace {in ptoas.cpp.ptoas_internal.hand each.cppkeeps only its TU-specific includes (net -232 duplicated lines).Not applicable:
borrowedContextctor /initializeMLIRContext/getMLIRContext(mutable-by-design semantics),pybind11_init__core(macro-generated),char **argv(C main ABI).Test plan
-Werror.ptoas_internal.hchain;git diff --checkclean.