Add --color_warnings to make warnings/errors like clang
This adds new (WARN|KATI_WARN|ERROR)_LOC log macro variants that take a
location as the first argument, and will prefix that location
information to the warning/error lines.
When --color_warnings is enabled, it reformats them to have a standard
warning:/error: infix, and adds colors in order to match the
warnings/errors produced by clang.
diff --git a/dep.cc b/dep.cc
index 4aebae0..75dff76 100644
--- a/dep.cc
+++ b/dep.cc
@@ -156,16 +156,18 @@
if (rules.empty()) {
is_double_colon = r->is_double_colon;
} else if (is_double_colon != r->is_double_colon) {
- ERROR("%s:%d: *** target file `%s' has both : and :: entries.",
- LOCF(r->loc), output.c_str());
+ ERROR_LOC(r->loc, "*** target file `%s' has both : and :: entries.",
+ output.c_str());
}
if (primary_rule && !r->cmds.empty() &&
!IsSuffixRule(output) && !r->is_double_colon) {
- WARN("%s:%d: warning: overriding commands for target `%s'",
- LOCF(r->cmd_loc()), output.c_str());
- WARN("%s:%d: warning: ignoring old commands for target `%s'",
- LOCF(primary_rule->cmd_loc()), output.c_str());
+ WARN_LOC(r->cmd_loc(),
+ "warning: overriding commands for target `%s'",
+ output.c_str());
+ WARN_LOC(primary_rule->cmd_loc(),
+ "warning: ignoring old commands for target `%s'",
+ output.c_str());
primary_rule = r;
}
if (!primary_rule && !r->cmds.empty()) {
@@ -274,8 +276,7 @@
if (targets.empty()) {
suffix_rules_.clear();
} else {
- WARN("%s:%d: kati doesn't support .SUFFIXES with prerequisites",
- LOCF(loc));
+ WARN_LOC(loc, "kati doesn't support .SUFFIXES with prerequisites");
}
}
@@ -296,7 +297,7 @@
};
for (const char** p = kUnsupportedBuiltinTargets; *p; p++) {
if (GetRuleInputs(Intern(*p), &targets, &loc)) {
- WARN("%s:%d: kati doesn't support %s", LOCF(loc), *p);
+ WARN_LOC(loc, "kati doesn't support %s", *p);
}
}
}
diff --git a/eval.cc b/eval.cc
index 4b56f1f..7a08be7 100644
--- a/eval.cc
+++ b/eval.cc
@@ -383,7 +383,7 @@
}
void Evaluator::Error(const string& msg) {
- ERROR("%s:%d: %s", LOCF(loc_), msg.c_str());
+ ERROR_LOC(loc_, "%s", msg.c_str());
}
unordered_set<Symbol> Evaluator::used_undefined_vars_;
diff --git a/expr.cc b/expr.cc
index 5e7afe6..30a25b5 100644
--- a/expr.cc
+++ b/expr.cc
@@ -317,9 +317,9 @@
f->AddArg(v);
i += n;
if (i == s.size()) {
- ERROR("%s:%d: *** unterminated call to function '%s': "
- "missing '%c'.",
- LOCF(loc), f->name(), terms[0]);
+ ERROR_LOC(loc, "*** unterminated call to function '%s': "
+ "missing '%c'.",
+ f->name(), terms[0]);
}
nargs++;
if (s[i] == terms[0]) {
@@ -332,8 +332,8 @@
}
if (nargs <= f->min_arity()) {
- ERROR("%s:%d: *** insufficient number of arguments (%d) to function `%s'.",
- LOCF(loc), nargs - 1, f->name());
+ ERROR_LOC(loc, "*** insufficient number of arguments (%d) to function `%s'.",
+ nargs - 1, f->name());
}
*index_out = i;
@@ -365,8 +365,8 @@
if (g_flags.enable_kati_warnings) {
size_t found = sym.str().find_first_of(" ({");
if (found != string::npos) {
- KATI_WARN("%s:%d: *warning*: variable lookup with '%c': %.*s",
- loc, sym.str()[found], SPF(s));
+ KATI_WARN_LOC(loc, "*warning*: variable lookup with '%c': %.*s",
+ sym.str()[found], SPF(s));
}
}
Value* r = new SymRef(sym);
@@ -386,8 +386,8 @@
ParseFunc(loc, func, s, i+1, terms, index_out);
return func;
} else {
- KATI_WARN("%s:%d: *warning*: unknown make function '%.*s': %.*s",
- loc, SPF(lit->val()), SPF(s));
+ KATI_WARN_LOC(loc, "*warning*: unknown make function '%.*s': %.*s",
+ SPF(lit->val()), SPF(s));
}
}
@@ -428,12 +428,12 @@
// for detail.
size_t found = s.find(cp);
if (found != string::npos) {
- KATI_WARN("%s:%d: *warning*: unmatched parentheses: %.*s",
- loc, SPF(s));
+ KATI_WARN_LOC(loc, "*warning*: unmatched parentheses: %.*s",
+ SPF(s));
*index_out = s.size();
return new SymRef(Intern(s.substr(2, found-2)));
}
- ERROR("%s:%d: *** unterminated variable reference.", LOCF(loc));
+ ERROR_LOC(loc, "*** unterminated variable reference.");
}
}
diff --git a/flags.cc b/flags.cc
index 3ad8e3c..5c14af7 100644
--- a/flags.cc
+++ b/flags.cc
@@ -97,6 +97,8 @@
detect_android_echo = true;
} else if (!strcmp(arg, "--detect_depfiles")) {
detect_depfiles = true;
+ } else if (!strcmp(arg, "--color_warnings")) {
+ color_warnings = true;
} else if (ParseCommandLineOptionWithArg(
"-j", argv, &i, &num_jobs_str)) {
num_jobs = strtol(num_jobs_str, NULL, 10);
diff --git a/flags.h b/flags.h
index d22d0cd..cd4090a 100644
--- a/flags.h
+++ b/flags.h
@@ -39,6 +39,7 @@
bool regen_debug;
bool regen_ignoring_kati_binary;
bool use_find_emulator;
+ bool color_warnings;
const char* goma_dir;
const char* ignore_dirty_pattern;
const char* no_ignore_dirty_pattern;
diff --git a/func.cc b/func.cc
index 716cb2b..2031319 100644
--- a/func.cc
+++ b/func.cc
@@ -466,8 +466,8 @@
string* text = new string;
args[0]->Eval(ev, text);
if (ev->avoid_io()) {
- KATI_WARN("%s:%d: *warning*: $(eval) in a recipe is not recommended: %s",
- LOCF(ev->loc()), text->c_str());
+ KATI_WARN_LOC(ev->loc(), "*warning*: $(eval) in a recipe is not recommended: %s",
+ text->c_str());
}
vector<Stmt*> stmts;
Parse(*text, ev->loc(), &stmts);
@@ -555,9 +555,9 @@
string cmd = args[0]->Eval(ev);
if (ev->avoid_io() && !HasNoIoInShellScript(cmd)) {
if (ev->eval_depth() > 1) {
- ERROR("%s:%d: kati doesn't support passing results of $(shell) "
- "to other make constructs: %s",
- LOCF(ev->loc()), cmd.c_str());
+ ERROR_LOC(ev->loc(), "kati doesn't support passing results of $(shell) "
+ "to other make constructs: %s",
+ cmd.c_str());
}
StripShellComment(&cmd);
*s += "$(";
@@ -595,8 +595,8 @@
const StringPiece func_name = TrimSpace(func_name_buf);
Var* func = ev->LookupVar(Intern(func_name));
if (!func->IsDefined()) {
- KATI_WARN("%s:%d: *warning*: undefined user function: %s",
- ev->loc(), func_name.as_string().c_str());
+ KATI_WARN_LOC(ev->loc(), "*warning*: undefined user function: %s",
+ func_name.as_string().c_str());
}
vector<unique_ptr<SimpleVar>> av;
for (size_t i = 1; i < args.size(); i++) {
@@ -676,8 +676,7 @@
StringPrintf("echo -e \"%s:%d: %s\" 2>&1", LOCF(ev->loc()), EchoEscape(a).c_str()));
return;
}
- printf("%s:%d: %s\n", LOCF(ev->loc()), a.c_str());
- fflush(stdout);
+ WARN_LOC(ev->loc(), "%s", a.c_str());
}
void ErrorFunc(const vector<Value*>& args, Evaluator* ev, string*) {
diff --git a/log.cc b/log.cc
index fb9fcd4..b7141f6 100644
--- a/log.cc
+++ b/log.cc
@@ -16,5 +16,46 @@
#include "log.h"
+#include "flags.h"
+#include "strutil.h"
+
+#define BOLD "\033[1m"
+#define RESET "\033[0m"
+#define MAGENTA "\033[35m"
+#define RED "\033[31m"
+
+void ColorErrorLog(const char* file, int line, const char* msg) {
+ if (file == nullptr) {
+ ERROR("%s", msg);
+ return;
+ }
+
+ if (g_flags.color_warnings) {
+ StringPiece filtered = TrimPrefix(msg, "*** ");
+
+ ERROR(BOLD "%s:%d: " RED "error: " RESET BOLD "%s" RESET,
+ file, line, filtered.as_string().c_str());
+ } else {
+ ERROR("%s:%d: %s", file, line, msg);
+ }
+}
+
+void ColorWarnLog(const char* file, int line, const char* msg) {
+ if (file == nullptr) {
+ fprintf(stderr, "%s\n", msg);
+ return;
+ }
+
+ if (g_flags.color_warnings) {
+ StringPiece filtered = TrimPrefix(msg, "*warning*: ");
+ filtered = TrimPrefix(filtered, "warning: ");
+
+ fprintf(stderr, BOLD "%s:%d: " MAGENTA "warning: " RESET BOLD "%s" RESET "\n",
+ file, line, filtered.as_string().c_str());
+ } else {
+ fprintf(stderr, "%s:%d: %s\n", file, line, msg);
+ }
+}
+
bool g_log_no_exit;
string* g_last_error;
diff --git a/log.h b/log.h
index 11cf0e5..58c733a 100644
--- a/log.h
+++ b/log.h
@@ -21,6 +21,7 @@
#include <string.h>
#include "flags.h"
+#include "log.h"
#include "stringprintf.h"
using namespace std;
@@ -73,4 +74,22 @@
#define CHECK(c) if (!(c)) ERROR("%s:%d: %s", __FILE__, __LINE__, #c)
+// Set of logging functions that will automatically colorize lines that have
+// location information when --color_warnings is set.
+void ColorWarnLog(const char* file, int line, const char *msg);
+void ColorErrorLog(const char* file, int line, const char *msg);
+
+#define WARN_LOC(loc, ...) do { \
+ ColorWarnLog(LOCF(loc), StringPrintf(__VA_ARGS__).c_str()); \
+ } while (0)
+
+#define KATI_WARN_LOC(loc, ...) do { \
+ if (g_flags.enable_kati_warnings) \
+ ColorWarnLog(LOCF(loc), StringPrintf(__VA_ARGS__).c_str()); \
+ } while(0)
+
+#define ERROR_LOC(loc, ...) do { \
+ ColorErrorLog(LOCF(loc), StringPrintf(__VA_ARGS__).c_str()); \
+ } while (0)
+
#endif // LOG_H_
diff --git a/main.cc b/main.cc
index 5afe2b1..a4df3c0 100644
--- a/main.cc
+++ b/main.cc
@@ -173,8 +173,8 @@
}
for (ParseErrorStmt* err : GetParseErrors()) {
- WARN("%s:%d: warning for parse error in an unevaluated line: %s",
- LOCF(err->loc()), err->msg.c_str());
+ WARN_LOC(err->loc(), "warning for parse error in an unevaluated line: %s",
+ err->msg.c_str());
}
vector<DepNode*> nodes;
diff --git a/parser.cc b/parser.cc
index 61d1ac5..3822dd8 100644
--- a/parser.cc
+++ b/parser.cc
@@ -92,10 +92,10 @@
}
if (!if_stack_.empty())
- ERROR("%s:%d: *** missing `endif'.", loc_.filename, loc_.lineno + 1);
+ ERROR_LOC(Loc(loc_.filename, loc_.lineno + 1), "*** missing `endif'.");
if (!define_name_.empty())
- ERROR("%s:%d: *** missing `endef', unterminated `define'.",
- loc_.filename, define_start_line_);
+ ERROR_LOC(Loc(loc_.filename, define_start_line_),
+ "*** missing `endef', unterminated `define'.");
}
static void Init() {
@@ -304,7 +304,7 @@
StringPiece rest = TrimRightSpace(RemoveComment(TrimLeftSpace(
line.substr(sizeof("endef")))));
if (!rest.empty()) {
- WARN("%s:%d: extraneous text after `endef' directive", LOCF(loc_));
+ WARN_LOC(loc_, "extraneous text after `endef' directive");
}
AssignStmt* stmt = new AssignStmt();
@@ -374,7 +374,7 @@
}
}
if (!s.empty()) {
- WARN("%s:%d: extraneous text after `ifeq' directive", LOCF(loc_));
+ WARN_LOC(loc_, "extraneous text after `ifeq' directive");
return true;
}
return true;
@@ -411,7 +411,7 @@
num_if_nest_ = st->num_nest + 1;
if (!HandleDirective(next_if, else_if_directives_)) {
- WARN("%s:%d: extraneous text after `else' directive", LOCF(loc_));
+ WARN_LOC(loc_, "extraneous text after `else' directive");
}
num_if_nest_ = 0;
}
diff --git a/rule.cc b/rule.cc
index 6942fb5..717d0f0 100644
--- a/rule.cc
+++ b/rule.cc
@@ -58,7 +58,7 @@
Rule** out_rule, RuleVarAssignment* rule_var) {
size_t index = line.find(':');
if (index == string::npos) {
- ERROR("%s:%d: *** missing separator.", LOCF(loc));
+ ERROR_LOC(loc, "*** missing separator.");
}
StringPiece first = line.substr(0, index);
@@ -71,8 +71,7 @@
!outputs.empty() && IsPatternRule(outputs[0].str()));
for (size_t i = 1; i < outputs.size(); i++) {
if (IsPatternRule(outputs[i].str()) != is_first_pattern) {
- ERROR("%s:%d: *** mixed implicit and normal rules: deprecated syntax",
- LOCF(loc));
+ ERROR_LOC(loc, "*** mixed implicit and normal rules: deprecated syntax");
}
}
@@ -94,8 +93,8 @@
// target specific variable).
// See https://github.com/google/kati/issues/83
if (term_index == 0) {
- KATI_WARN("%s:%d: defining a target which starts with `=', "
- "which is not probably what you meant", LOCF(loc));
+ KATI_WARN_LOC(loc, "defining a target which starts with `=', "
+ "which is not probably what you meant");
buf = line.as_string();
if (term)
buf += term;
@@ -136,8 +135,7 @@
}
if (is_first_pattern) {
- ERROR("%s:%d: *** mixed implicit and normal rules: deprecated syntax",
- LOCF(loc));
+ ERROR_LOC(loc, "*** mixed implicit and normal rules: deprecated syntax");
}
StringPiece second = rest.substr(0, index);
@@ -147,8 +145,8 @@
tok = TrimLeadingCurdir(tok);
for (Symbol output : rule->outputs) {
if (!Pattern(tok).Match(output.str())) {
- WARN("%s:%d: target `%s' doesn't match the target pattern",
- LOCF(loc), output.c_str());
+ WARN_LOC(loc, "target `%s' doesn't match the target pattern",
+ output.c_str());
}
}
@@ -156,13 +154,13 @@
}
if (rule->output_patterns.empty()) {
- ERROR("%s:%d: *** missing target pattern.", LOCF(loc));
+ ERROR_LOC(loc, "*** missing target pattern.");
}
if (rule->output_patterns.size() > 1) {
- ERROR("%s:%d: *** multiple target patterns.", LOCF(loc));
+ ERROR_LOC(loc, "*** multiple target patterns.");
}
if (!IsPatternRule(rule->output_patterns[0].str())) {
- ERROR("%s:%d: *** target pattern contains no '%%'.", LOCF(loc));
+ ERROR_LOC(loc, "*** target pattern contains no '%%'.");
}
ParseInputs(rule, third);
}
diff --git a/rule.h b/rule.h
index 84412c1..48394bb 100644
--- a/rule.h
+++ b/rule.h
@@ -49,7 +49,7 @@
private:
void Error(const string& msg) {
- ERROR("%s:%d: %s", loc.filename, loc.lineno, msg.c_str());
+ ERROR_LOC(loc, "%s", msg.c_str());
}
};
diff --git a/runtest.rb b/runtest.rb
index fc594d5..190c11f 100755
--- a/runtest.rb
+++ b/runtest.rb
@@ -170,7 +170,7 @@
output.gsub!(/\/bin\/sh: ([^:]*): command not found/,
"\\1: Command not found")
output.gsub!(/.*: warning for parse error in an unevaluated line: .*\n/, '')
- output.gsub!(/^FindEmulator: /, '')
+ output.gsub!(/^([^ ]+: )?FindEmulator: /, '')
output.gsub!(/^\/bin\/sh: line 0: /, '')
output.gsub!(/ (\.\/+)+kati\.\S+/, '') # kati log files in find_command.mk
output.gsub!(/ (\.\/+)+test\S+.json/, '') # json files in find_command.mk
diff --git a/strutil.cc b/strutil.cc
index b094198..632d151 100644
--- a/strutil.cc
+++ b/strutil.cc
@@ -174,6 +174,13 @@
return true;
}
+StringPiece TrimPrefix(StringPiece str, StringPiece prefix) {
+ ssize_t size_diff = str.size() - prefix.size();
+ if (size_diff < 0 || str.substr(0, prefix.size()) != prefix)
+ return str;
+ return str.substr(prefix.size());
+}
+
StringPiece TrimSuffix(StringPiece str, StringPiece suffix) {
ssize_t size_diff = str.size() - suffix.size();
if (size_diff < 0 || str.substr(size_diff) != suffix)
diff --git a/strutil.h b/strutil.h
index d61160d..ff3d6a1 100644
--- a/strutil.h
+++ b/strutil.h
@@ -89,6 +89,8 @@
bool HasWord(StringPiece str, StringPiece w);
+StringPiece TrimPrefix(StringPiece str, StringPiece suffix);
+
StringPiece TrimSuffix(StringPiece str, StringPiece suffix);
class Pattern {
diff --git a/strutil_test.cc b/strutil_test.cc
index 7a5a47d..a26cf4d 100644
--- a/strutil_test.cc
+++ b/strutil_test.cc
@@ -56,6 +56,20 @@
assert(!HasSuffix("bar", "bbar"));
}
+void TestTrimPrefix() {
+ ASSERT_EQ(TrimPrefix("foo", "foo"), "");
+ ASSERT_EQ(TrimPrefix("foo", "fo"), "o");
+ ASSERT_EQ(TrimPrefix("foo", ""), "foo");
+ ASSERT_EQ(TrimPrefix("foo", "fooo"), "foo");
+}
+
+void TestTrimSuffix() {
+ ASSERT_EQ(TrimSuffix("bar", "bar"), "");
+ ASSERT_EQ(TrimSuffix("bar", "ar"), "b");
+ ASSERT_EQ(TrimSuffix("bar", ""), "bar");
+ ASSERT_EQ(TrimSuffix("bar", "bbar"), "bar");
+}
+
string SubstPattern(StringPiece str, StringPiece pat, StringPiece subst) {
string r;
Pattern(pat).AppendSubst(str, subst, &r);
@@ -190,6 +204,8 @@
TestWordScanner();
TestHasPrefix();
TestHasSuffix();
+ TestTrimPrefix();
+ TestTrimSuffix();
TestSubstPattern();
TestNoLineBreak();
TestHasWord();
