-
Notifications
You must be signed in to change notification settings - Fork 3
Remove grammar import mechanism, preserve external symbol mechanism #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: copilot/revert-grammar-import-mechanism
Are you sure you want to change the base?
Remove grammar import mechanism, preserve external symbol mechanism #2
Conversation
Co-authored-by: godgnidoc <[email protected]>
Co-authored-by: godgnidoc <[email protected]>
Co-authored-by: godgnidoc <[email protected]>
Co-authored-by: godgnidoc <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR successfully separates the grammar import mechanism from the external symbol mechanism, removing the former while preserving the latter. The changes enable embedding other languages as external symbols in grammars without the complexity of importing individual lexical/syntactical units.
Key changes:
- Removed
usingstatements and relatedterm.using/ntrm.usingrules from grammar definition - Preserved
ntrm.externalfor loading external grammars as symbols - Added
ExternalLoadercallback type to support external symbol loading viasyntax.Import()
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
grammar/grammar.grammar |
Removed USING terminal and using/usings rules; simplified term to single pattern; kept ntrm.external for external symbols |
include/aliox/grammar.h |
Added ExternalLoader callback type and overloaded Compile() method to accept external loader |
src/aliox/grammar.cpp |
Implemented external symbol loading logic; simplified term/ntrm processing by removing import-related branches |
generated/include/grammar/syntax.h |
Updated hardcoded syntax to match simplified grammar; removed Using struct and related code; adjusted symbol/formula IDs |
include/alioth/println_compat.h |
Added compatibility shim for fmt::println on fmt < 10.0.0 |
src/alioth/syntax.cpp, src/alioth/parser.cpp, src/aliox/template.cpp, src/alioth-cli/skeleton.cpp, src/alioth-cli/parse.cpp |
Added println_compat.h include for fmt compatibility |
.gitignore |
Added _codeql_build_dir/ to ignored paths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| inline void println(FILE* f, const S& format, Args&&... args) { | ||
| print(f, fmt::runtime(format), std::forward<Args>(args)...); | ||
| std::fputc('\n', f); | ||
| } | ||
|
|
||
| template <typename S, typename... Args> | ||
| inline void println(const S& format, Args&&... args) { | ||
| print(fmt::runtime(format), std::forward<Args>(args)...); | ||
| std::putchar('\n'); | ||
| } |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compatibility implementation unconditionally wraps format strings with fmt::runtime(), which bypasses compile-time format string validation even for string literals. This should only be used for runtime format strings. Consider removing fmt::runtime() wrapper to preserve compile-time checking:
template <typename S, typename... Args>
inline void println(FILE* f, const S& format, Args&&... args) {
print(f, format, std::forward<Args>(args)...);
std::fputc('\n', f);
}
template <typename S, typename... Args>
inline void println(const S& format, Args&&... args) {
print(format, std::forward<Args>(args)...);
std::putchar('\n');
}
The previous work to remove the import mechanism inadvertently broke the external symbol mechanism. This PR cleanly separates the two:
usingstatements,term.using,ntrm.usingfor importing lexical/syntactical units from other grammar filesntrm.externalfor embedding other languages as symbols in the current grammarChanges
Grammar definition (
grammar/grammar.grammar)USINGterminal,using/usingsrules,term.using,ntrm.usingntrm.external -> ID@name ARROW STRING@grammar SEMICOLON;termto single pattern (wasterm.define/term.using)Grammar loader (
src/aliox/grammar.cpp,include/aliox/grammar.h)ExternalLoadercallback type for loading external grammarssyntax.Import()Hardcoded syntax (
generated/include/grammar/syntax.h)Build compatibility
println_compat.hfor fmt < 10.0.0 compatibilityWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
center2.conan.io/home/REDACTED/.local/bin/conan conan install /tmp/conanfile.txt --output-folder=/tmp/conan_deps --build=missing(dns block)esm.ubuntu.com/usr/lib/apt/methods/https /usr/lib/apt/methods/https(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.