mirror of
https://github.com/Z3Prover/z3
synced 2026-01-28 21:08:43 +00:00
Disable std::optional refactoring in code-conventions-analyzer workflow (#8349)
* Initial plan * Remove std::optional refactoring instructions from code-conventions-analyzer Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com> * Recompile workflows after removing std::optional instructions Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com> * Update workflow lock files to latest version Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
This commit is contained in:
parent
76d46ee48a
commit
4d86525319
8 changed files with 124 additions and 481 deletions
223
.github/workflows/code-conventions-analyzer.lock.yml
generated
vendored
223
.github/workflows/code-conventions-analyzer.lock.yml
generated
vendored
|
|
@ -13,7 +13,7 @@
|
|||
# \ /\ / (_) | | | | ( | | | | (_) \ V V /\__ \
|
||||
# \/ \/ \___/|_| |_|\_\|_| |_|\___/ \_/\_/ |___/
|
||||
#
|
||||
# This file was automatically generated by gh-aw (v0.37.22). DO NOT EDIT.
|
||||
# This file was automatically generated by gh-aw (v0.37.23). DO NOT EDIT.
|
||||
#
|
||||
# To update this file, edit the corresponding .md file and run:
|
||||
# gh aw compile
|
||||
|
|
@ -44,7 +44,7 @@ jobs:
|
|||
comment_repo: ""
|
||||
steps:
|
||||
- name: Setup Scripts
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.22
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.23
|
||||
with:
|
||||
destination: /opt/gh-aw/actions
|
||||
- name: Check workflow file timestamps
|
||||
|
|
@ -81,7 +81,7 @@ jobs:
|
|||
secret_verification_result: ${{ steps.validate-secret.outputs.verification_result }}
|
||||
steps:
|
||||
- name: Setup Scripts
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.22
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.23
|
||||
with:
|
||||
destination: /opt/gh-aw/actions
|
||||
- name: Checkout repository
|
||||
|
|
@ -492,7 +492,7 @@ jobs:
|
|||
model: process.env.GH_AW_MODEL_AGENT_COPILOT || "",
|
||||
version: "",
|
||||
agent_version: "0.0.394",
|
||||
cli_version: "v0.37.22",
|
||||
cli_version: "v0.37.23",
|
||||
workflow_name: "Code Conventions Analyzer",
|
||||
experimental: false,
|
||||
supports_tools_allowlist: true,
|
||||
|
|
@ -603,24 +603,9 @@ jobs:
|
|||
|
||||
## Your Task
|
||||
|
||||
**PRIMARY FOCUS: Create Issues for std::optional Refactoring**
|
||||
**PRIMARY FOCUS: Create Issues for Tuple Pattern (Structured Bindings) Refactoring**
|
||||
|
||||
Your primary task is to identify and **directly implement** refactorings that replace pointer-based optional patterns with `std::optional<T>`. This workflow will:
|
||||
|
||||
1. **Find std::optional opportunities** - Functions returning null pointers to indicate absence or using output parameters
|
||||
2. **Implement the refactoring** - Use the `edit` tool to make actual code changes
|
||||
3. **Create issues** - Automatically create an issue with your changes for std::optional improvements
|
||||
4. **Create discussions for other findings** - For other code quality issues, create discussions (not issues)
|
||||
|
||||
**Focus Areas for std::optional Refactoring:**
|
||||
- Functions returning `nullptr` to indicate "no value"
|
||||
- Functions using output parameters (pointer/reference parameters) to return optional results
|
||||
- Boolean return + output parameter patterns (e.g., `bool get_value(T* out)`)
|
||||
- APIs that would benefit from explicit optional semantics
|
||||
|
||||
**SECONDARY FOCUS: Create Issues for Tuple Pattern (Structured Bindings) Refactoring**
|
||||
|
||||
Your secondary task is to identify and implement refactorings that use C++17 structured bindings instead of accessing `.first` and `.second`:
|
||||
Your primary task is to identify and implement refactorings that use C++17 structured bindings instead of accessing `.first` and `.second`:
|
||||
|
||||
1. **Find tuple/pair access patterns** - Code accessing `.first` and `.second` members
|
||||
2. **Implement the refactoring** - Replace with structured bindings for clearer code
|
||||
|
|
@ -650,9 +635,9 @@ jobs:
|
|||
return merge(n1, n2);
|
||||
```
|
||||
|
||||
**TERTIARY FOCUS: Create Issues for initializer_list Refactoring**
|
||||
**SECONDARY FOCUS: Create Issues for initializer_list Refactoring**
|
||||
|
||||
Your tertiary task is to identify and implement refactorings that use `std::initializer_list<T>` instead of array pointer + size parameters:
|
||||
Your secondary task is to identify and implement refactorings that use `std::initializer_list<T>` instead of array pointer + size parameters:
|
||||
|
||||
1. **Find array + size parameter patterns** - Functions taking `(unsigned sz, T* args)` or similar
|
||||
2. **Implement the refactoring** - Replace with `std::initializer_list<T>` for cleaner APIs
|
||||
|
|
@ -692,127 +677,7 @@ jobs:
|
|||
Additionally, conduct analysis of other coding conventions and modern C++ opportunities for discussion (not immediate implementation)
|
||||
|
||||
|
||||
## Workflow for std::optional Refactoring (PRIMARY)
|
||||
|
||||
### Step A: Find std::optional Refactoring Opportunities
|
||||
|
||||
1. **Search for common patterns** that should use `std::optional`:
|
||||
```bash
|
||||
# Functions returning nullptr to indicate absence
|
||||
grep pattern: "return nullptr;" glob: "src/**/*.{cpp,h}"
|
||||
|
||||
# Boolean return + output parameter patterns
|
||||
grep pattern: "bool [a-z_]+\(.*\*" glob: "src/**/*.h"
|
||||
grep pattern: "bool [a-z_]+\(.*&" glob: "src/**/*.h"
|
||||
|
||||
# Functions with output parameters
|
||||
grep pattern: "\([^,]+\*[^,]*\)" glob: "src/**/*.h"
|
||||
```
|
||||
|
||||
2. **Analyze candidates** for refactoring:
|
||||
- Use `view` to examine the function implementation
|
||||
- Check if the function is part of the public API or internal
|
||||
- Verify that the pattern is indeed optional (not always valid)
|
||||
- Ensure the change would improve code clarity
|
||||
|
||||
3. **Select 1-3 high-value targets** per run:
|
||||
- Prefer internal APIs over public APIs (less breaking)
|
||||
- Choose functions with clear optional semantics
|
||||
- Focus on functions with multiple call sites for broader impact
|
||||
|
||||
### Step B: Implement the Refactoring
|
||||
|
||||
For each selected function:
|
||||
|
||||
1. **Update the function signature** in header file:
|
||||
```cpp
|
||||
// Before:
|
||||
bool get_something(T* result);
|
||||
// or
|
||||
T* find_something();
|
||||
|
||||
// After:
|
||||
std::optional<T> get_something();
|
||||
```
|
||||
|
||||
2. **Update the function implementation**:
|
||||
```cpp
|
||||
// Before:
|
||||
bool get_something(T* result) {
|
||||
if (condition) {
|
||||
*result = value;
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// After:
|
||||
std::optional<T> get_something() {
|
||||
if (condition) {
|
||||
return value;
|
||||
}
|
||||
return std::nullopt;
|
||||
}
|
||||
```
|
||||
|
||||
3. **Update all call sites** to use the new API:
|
||||
```cpp
|
||||
// Before:
|
||||
T result;
|
||||
if (get_something(&result)) {
|
||||
use(result);
|
||||
}
|
||||
|
||||
// After:
|
||||
if (auto result = get_something()) {
|
||||
use(*result);
|
||||
}
|
||||
```
|
||||
|
||||
4. **Verify the changes**:
|
||||
- Use `grep` to find any remaining call sites
|
||||
- Check that the refactoring is complete
|
||||
- Ensure no compilation errors would occur
|
||||
|
||||
### Step C: Create the Issue
|
||||
|
||||
Use the `output.create-issue` tool to create an issue with:
|
||||
- **Title**: "Refactor [function_name] to use std::optional"
|
||||
- **Description**:
|
||||
- Explain what was changed
|
||||
- Why std::optional is better (type safety, explicit semantics)
|
||||
- List all modified files
|
||||
- Note any caveats or considerations
|
||||
|
||||
**Example issue description:**
|
||||
```markdown
|
||||
# Refactor to use std::optional
|
||||
|
||||
This PR refactors the following functions to use `std::optional<T>` instead of pointer-based optional patterns:
|
||||
|
||||
- `get_value()` in `src/util/some_file.cpp`
|
||||
- `find_item()` in `src/ast/another_file.cpp`
|
||||
|
||||
## Benefits:
|
||||
- Explicit optional semantics (no nullptr checks needed)
|
||||
- Type safety (can't forget to check for absence)
|
||||
- Modern C++17 idiom
|
||||
|
||||
## Changes:
|
||||
- Updated function signatures to return `std::optional<T>`
|
||||
- Modified implementations to return `std::nullopt` instead of `nullptr`
|
||||
- Updated all call sites to use optional idioms
|
||||
|
||||
## Testing:
|
||||
- No functional changes to logic
|
||||
- All existing call sites updated
|
||||
```
|
||||
|
||||
### Step D: Create Discussion for Other Findings
|
||||
|
||||
If you identify other code quality issues (naming, formatting, other C++ features), create a **discussion** (not a PR) with those findings using the existing discussion format from the workflow.
|
||||
|
||||
## Workflow for initializer_list Refactoring (TERTIARY)
|
||||
## Workflow for initializer_list Refactoring (SECONDARY)
|
||||
|
||||
### Step A: Find initializer_list Refactoring Opportunities
|
||||
|
||||
|
|
@ -1010,7 +875,6 @@ jobs:
|
|||
**C++17 features:**
|
||||
- Structured bindings for tuple/pair unpacking
|
||||
- `if constexpr` for compile-time conditionals
|
||||
- **`std::optional` instead of pointer-based optional values** - **PRIMARY FOCUS: Implement these changes directly (see "Workflow for std::optional Refactoring" section near the beginning of this document)**
|
||||
- `std::string_view` for string parameters
|
||||
- Fold expressions for variadic templates
|
||||
- `[[nodiscard]]` and `[[maybe_unused]]` attributes
|
||||
|
|
@ -1042,8 +906,6 @@ jobs:
|
|||
- Constructors with member initializers (may still be candidates for improvement)
|
||||
- Constructors that only initialize members to default values
|
||||
- **Empty destructors**: Trivial destructors that can be removed or use `= default`
|
||||
PROMPT_EOF
|
||||
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
||||
- Destructors with empty body `~Class() {}`
|
||||
- Non-virtual destructors that don't need to be explicitly defined
|
||||
- Virtual destructors (keep explicit even if empty for polymorphic classes),
|
||||
|
|
@ -1105,13 +967,8 @@ jobs:
|
|||
- Incorrect usage of `std::move` (moving from const references, etc.)
|
||||
- Return value optimization opportunities being blocked
|
||||
|
||||
**Optional Value Patterns:**
|
||||
- **PRIMARY TASK**: Functions returning null + using output parameters
|
||||
- **ACTION**: Replace with `std::optional<T>` return values using the refactoring workflow above
|
||||
- **RESULT**: Create an issue with the actual code changes (see "Workflow for std::optional Refactoring")
|
||||
|
||||
**Tuple/Pair Access Patterns:**
|
||||
- **SECONDARY TASK**: Code accessing `.first` and `.second` on pairs/tuples
|
||||
- **PRIMARY TASK**: Code accessing `.first` and `.second` on pairs/tuples
|
||||
- **ACTION**: Replace with C++17 structured bindings for cleaner, more readable code
|
||||
- **RESULT**: Create an issue with the actual code changes
|
||||
- **NAMING**: Use descriptive names based on types/semantics (e.g., `[n1, n2]` for `enode_pair`, `[k, v]` for maps)
|
||||
|
|
@ -1162,6 +1019,8 @@ jobs:
|
|||
## Analysis Methodology
|
||||
|
||||
1. **Sample key directories** in the codebase:
|
||||
PROMPT_EOF
|
||||
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
||||
- `src/util/` - Core utilities and data structures
|
||||
- `src/ast/` - Abstract syntax tree implementations
|
||||
- `src/smt/` - SMT solver core
|
||||
|
|
@ -1205,16 +1064,16 @@ jobs:
|
|||
|
||||
## Deliverables
|
||||
|
||||
### PRIMARY: Issue for std::optional Refactoring
|
||||
### PRIMARY: Issues for Code Refactoring
|
||||
|
||||
If you implement std::optional refactoring (following the workflow above), create an issue using `output.create-issue` with:
|
||||
When you implement refactorings (structured bindings, initializer_list), create issues using `output.create-issue` with:
|
||||
- Clear title indicating what was refactored
|
||||
- Description of changes and benefits
|
||||
- List of modified files and functions
|
||||
|
||||
### SECONDARY: Detailed Analysis Discussion
|
||||
|
||||
For other code quality findings (non-std::optional), create a comprehensive discussion with your findings structured as follows:
|
||||
For other code quality findings, create a comprehensive discussion with your findings structured as follows:
|
||||
|
||||
### Discussion Title
|
||||
"Code Conventions Analysis - [Date] - [Key Finding Summary]"
|
||||
|
|
@ -1435,30 +1294,14 @@ jobs:
|
|||
- **Simplification**: [Code that would be cleaner with auto cleanup]
|
||||
|
||||
### 4.7 Move Semantics Analysis
|
||||
PROMPT_EOF
|
||||
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
||||
- **Missing std::move**: [Returns/assignments that should use move]
|
||||
- **Incorrect std::move**: [Move from const, unnecessary moves]
|
||||
- **Return Value Optimization**: [Places where RVO is blocked]
|
||||
|
||||
### 4.8 Optional Value Pattern Modernization - **IMPLEMENT AS ISSUE**
|
||||
### 4.8 Tuple Pattern (Structured Bindings) Modernization - **IMPLEMENT AS ISSUE**
|
||||
|
||||
**This is the PRIMARY focus area - implement these changes directly:**
|
||||
|
||||
- **Current Pattern**: Functions returning null + output parameters
|
||||
- **Modern Pattern**: `std::optional<T>` return value opportunities
|
||||
- **Action**: Use the "Workflow for std::optional Refactoring" section above to:
|
||||
1. Find candidate functions
|
||||
2. Refactor using the `edit` tool
|
||||
3. Create an issue with your changes
|
||||
- **API Improvements**: Specific function signatures to update
|
||||
- **Examples**: File:line references with before/after code
|
||||
- **Output**: Issue (not just discussion)
|
||||
|
||||
### 4.9 Tuple Pattern (Structured Bindings) Modernization - **IMPLEMENT AS ISSUE**
|
||||
|
||||
**This is a SECONDARY focus area - implement these changes directly:**
|
||||
|
||||
- **Current Pattern**: Accessing `.first` and `.second` on pairs/tuples
|
||||
- **Modern Pattern**: Use C++17 structured bindings for cleaner code
|
||||
- **Benefits**:
|
||||
|
|
@ -1514,21 +1357,21 @@ jobs:
|
|||
- Have sequential uses of both `.first` and `.second`
|
||||
- **Output**: Issue with refactored code
|
||||
|
||||
### 4.10 Exception String Construction
|
||||
### 4.9 Exception String Construction
|
||||
- **Current**: [stringstream usage for building exception messages]
|
||||
- **Modern**: [std::format and std::formater opportunities]
|
||||
- **String Copies**: [Unnecessary copies when raising exceptions]
|
||||
- **Examples**: [Specific exception construction sites]
|
||||
|
||||
### 4.11 Array Parameter Modernization (std::span)
|
||||
### 4.10 Array Parameter Modernization (std::span)
|
||||
- **Current**: [Pointer + size parameter pairs for runtime-sized arrays]
|
||||
- **Modern**: [std::span usage opportunities]
|
||||
- **Type Safety**: [How span improves API safety]
|
||||
- **Examples**: [Function signatures to update]
|
||||
|
||||
### 4.12 Array Parameter Modernization (std::initializer_list) - **IMPLEMENT AS ISSUE**
|
||||
### 4.11 Array Parameter Modernization (std::initializer_list) - **IMPLEMENT AS ISSUE**
|
||||
|
||||
**This is a TERTIARY focus area - implement these changes directly:**
|
||||
**This is a SECONDARY focus area - implement these changes directly:**
|
||||
|
||||
- **Current Pattern**: Functions with `unsigned sz, T* args` or `unsigned sz, T* const* args` parameters
|
||||
- **Modern Pattern**: Use `std::initializer_list<T>` for functions called with compile-time constant arrays
|
||||
|
|
@ -1578,18 +1421,20 @@ jobs:
|
|||
- **Output**: Issue with refactored code
|
||||
- **Note**: Only apply to internal C++ APIs, not to public C API functions that need C compatibility
|
||||
|
||||
### 4.13 Increment Operator Patterns
|
||||
### 4.12 Increment Operator Patterns
|
||||
- **Postfix Usage**: [Count of i++ where result is unused]
|
||||
- **Prefix Preference**: [Places to use ++i instead]
|
||||
PROMPT_EOF
|
||||
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
||||
- **Iterator Loops**: [Heavy iterator usage areas]
|
||||
|
||||
### 4.14 Exception Control Flow
|
||||
### 4.13 Exception Control Flow
|
||||
- **Current Usage**: [Exceptions used for normal control flow]
|
||||
- **Modern Alternatives**: [std::expected, std::optional, error codes]
|
||||
- **Modern Alternatives**: [std::expected or error codes]
|
||||
- **Performance**: [Impact of exception-based control flow]
|
||||
- **Refactoring Opportunities**: [Specific patterns to replace]
|
||||
|
||||
### 4.15 Inefficient Stream Output
|
||||
### 4.14 Inefficient Stream Output
|
||||
- **Current Usage**: [string stream output operator used for single characters]
|
||||
- **Modern Alternatives**: [use char output operator]
|
||||
- **Performance**: [Reduce code size and improve performance]
|
||||
|
|
@ -1894,17 +1739,15 @@ jobs:
|
|||
|
||||
- Never execute untrusted code
|
||||
- Use `bash` only for safe operations (git, grep patterns)
|
||||
PROMPT_EOF
|
||||
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
||||
- **For std::optional refactoring**: Use the `edit` tool to modify files directly
|
||||
- **For code refactoring (structured bindings, initializer_list)**: Use the `edit` tool to modify files directly
|
||||
- **For other findings**: Create discussions only (no code modifications)
|
||||
- All code changes for std::optional will be reviewed through the issue process
|
||||
- All code changes will be reviewed through the issue process
|
||||
|
||||
## Output Requirements
|
||||
|
||||
**Two types of outputs:**
|
||||
|
||||
1. **Issue** (for std::optional refactoring):
|
||||
1. **Issue** (for refactorings like structured bindings or initializer_list):
|
||||
- Use `output.create-issue` to create an issue
|
||||
- Include clear title and description
|
||||
- List all modified files
|
||||
|
|
@ -2157,7 +2000,7 @@ jobs:
|
|||
total_count: ${{ steps.missing_tool.outputs.total_count }}
|
||||
steps:
|
||||
- name: Setup Scripts
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.22
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.23
|
||||
with:
|
||||
destination: /opt/gh-aw/actions
|
||||
- name: Debug job inputs
|
||||
|
|
@ -2258,7 +2101,7 @@ jobs:
|
|||
success: ${{ steps.parse_results.outputs.success }}
|
||||
steps:
|
||||
- name: Setup Scripts
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.22
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.23
|
||||
with:
|
||||
destination: /opt/gh-aw/actions
|
||||
- name: Download agent artifacts
|
||||
|
|
@ -2409,7 +2252,7 @@ jobs:
|
|||
process_safe_outputs_temporary_id_map: ${{ steps.process_safe_outputs.outputs.temporary_id_map }}
|
||||
steps:
|
||||
- name: Setup Scripts
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.22
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.23
|
||||
with:
|
||||
destination: /opt/gh-aw/actions
|
||||
- name: Download agent output artifact
|
||||
|
|
@ -2446,7 +2289,7 @@ jobs:
|
|||
permissions: {}
|
||||
steps:
|
||||
- name: Setup Scripts
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.22
|
||||
uses: githubnext/gh-aw/actions/setup@v0.37.23
|
||||
with:
|
||||
destination: /opt/gh-aw/actions
|
||||
- name: Download cache-memory artifact (default)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue