-
Notifications
You must be signed in to change notification settings - Fork 146
Description
Summary
Add optional compiler warnings for identifiers that don't follow configurable naming conventions. This would help projects using Sail maintain consistent code style.
Motivation
The sail-riscv project currently has a mix of TitleCase, snake_case, and camelCase for various identifier types (types, functions, variables). There is an open issue requesting standardization:
Having compiler-level lints would provide a systematic way to enforce naming conventions, rather than relying solely on code review or external scripts.
Proposed Solution
Add a CLI flag (e.g., --warn-naming-convention or -Wnaming) that enables naming convention checks during compilation. The compiler would emit warnings (not errors) for non-conforming identifiers.
Suggested Default Conventions
| Identifier Type | Expected Style | Example |
|---|---|---|
| Types / Structs / Enums | TitleCase |
MemoryAccess, Privilege |
| Functions | snake_case |
execute_load, get_csr_value |
| Variables / Let bindings | snake_case |
mem_addr, reg_value |
| Constants | SCREAMING_SNAKE_CASE |
MAX_XLEN, DEFAULT_VALUE |
Example Output
$ sail --warn-naming-convention model.sail
model.sail:42:0: warning: type 'memory_access' should use TitleCase
model.sail:87:0: warning: function 'GetValue' should use snake_case
Possible Implementation Approaches
Option 1: Built-in Conventions (Simpler)
Hardcode the naming conventions listed above. This covers the most common use case.
Option 2: Configurable via CLI (More Flexible)
Allow users to specify patterns per identifier type:
sail --naming-types=TitleCase --naming-functions=snake_case model.sail
Option 3: Configuration File (Most Flexible)
Support a .sail-lint or similar config file:
{
"naming": {
"types": "TitleCase",
"functions": "snake_case",
"variables": "snake_case",
"constants": "SCREAMING_SNAKE_CASE"
}
}Implementation Notes
Based on a preliminary review of the codebase, the check could be implemented by:
- Adding a new module (e.g.,
naming_check.ml) that traverses the AST - Checking identifiers in
DEF_type,DEF_fundef,DEF_val, etc. - Using
Reporting.warnto emit warnings - Adding a CLI flag in
sail.mlto enable/disable the feature
I would be happy to contribute a PR if this feature is accepted and you can provide guidance on the preferred approach.
Additional Context
- This feature would benefit all Sail users, not just sail-riscv
- Warnings (not errors) ensure backward compatibility
- The feature should be opt-in to avoid breaking existing workflows
Related Issues
- sail-riscv naming convention discussion: Normalise case style riscv/sail-riscv#1293