Skip to content

Commit 2b4a2c7

Browse files
authored
Fix some clang-format lints (#57)
* address some clang-format lints * Fix tool response polyfill regression * Fix more lints * rm -exclude-header-filter * disable clang-analyzer-cplusplus.StringChecker
1 parent 8a76f78 commit 2b4a2c7

File tree

5 files changed

+145
-100
lines changed

5 files changed

+145
-100
lines changed

CMakeLists.txt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,22 @@ project(minja VERSION 1.0.0 LANGUAGES CXX)
1313

1414
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
1515

16+
# Test if clang-tidy is available
17+
find_program(CLANG_TIDY_EXE NAMES "clang-tidy")
18+
if (CLANG_TIDY_EXE)
19+
message(STATUS "clang-tidy found: ${CLANG_TIDY_EXE}")
20+
set(CMAKE_CXX_CLANG_TIDY
21+
clang-tidy;
22+
-header-filter=include/minja/.*;
23+
# https://clang.llvm.org/extra/clang-tidy/checks/list.html
24+
# TODO: enable more / disable less checks: google-*,misc-*,modernize-*,performance-*
25+
-checks=-*,clang-analyzer-*,clang-diagnostic-*,cppcoreguideline-*,bugprone-*,-bugprone-suspicious-include,-bugprone-assignment-in-if-condition,-bugprone-narrowing-conversions,-bugprone-easily-swappable-parameters,-bugprone-inc-dec-in-conditions,-bugprone-exception-escape,-clang-analyzer-cplusplus.StringChecker;
26+
-warnings-as-errors=*;
27+
)
28+
else()
29+
message(STATUS "clang-tidy not found")
30+
endif()
31+
1632
if (MSVC)
1733
set(MINJA_FUZZTEST_ENABLED_DEFAULT OFF)
1834
set(MINJA_USE_VENV_DEFAULT OFF)

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ Main limitations (non-exhaustive list):
166166
ctest --test-dir build -j --output-on-failure
167167
```
168168

169+
- Bonus: install `clang-tidy` before building (on MacOS: `brew install llvm ; sudo ln -s "$(brew --prefix llvm)/bin/clang-tidy" "/usr/local/bin/clang-tidy"`)
170+
169171
- Fuzzing tests
170172

171173
- Note: `fuzztest` **[doesn't work](https://github.com/google/fuzztest/issues/179)** natively on Windows or MacOS.

include/minja/chat-template.hpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@
1010

1111
#include "minja.hpp"
1212

13+
#include <chrono>
14+
#include <cstddef>
15+
#include <cstdio>
16+
#include <exception>
17+
#include <iomanip>
18+
#include <memory>
19+
#include <sstream>
1320
#include <string>
1421
#include <vector>
1522

@@ -427,7 +434,7 @@ class chat_template {
427434
auto obj = json {
428435
{"tool_calls", tool_calls},
429436
};
430-
if (!content.is_null() && content != "") {
437+
if (!content.is_null() && !content.empty()) {
431438
obj["content"] = content;
432439
}
433440
message["content"] = obj.dump(2);
@@ -437,13 +444,12 @@ class chat_template {
437444
if (polyfill_tool_responses && role == "tool") {
438445
message["role"] = "user";
439446
auto obj = json {
440-
{"tool_response", {
441-
{"content", message.at("content")},
442-
}},
447+
{"tool_response", json::object()},
443448
};
444449
if (message.contains("name")) {
445-
obj["tool_response"]["name"] = message.at("name");
450+
obj["tool_response"]["tool"] = message.at("name");
446451
}
452+
obj["tool_response"]["content"] = message.at("content");
447453
if (message.contains("tool_call_id")) {
448454
obj["tool_response"]["tool_call_id"] = message.at("tool_call_id");
449455
}
@@ -512,7 +518,7 @@ class chat_template {
512518
static nlohmann::ordered_json add_system(const nlohmann::ordered_json & messages, const std::string & system_prompt) {
513519
json messages_with_system = messages;
514520

515-
if (messages_with_system.size() > 0 && messages_with_system[0].at("role") == "system") {
521+
if (!messages_with_system.empty() && messages_with_system[0].at("role") == "system") {
516522
std::string existing_system = messages_with_system.at(0).at("content");
517523
messages_with_system[0] = json {
518524
{"role", "system"},

0 commit comments

Comments
 (0)