From 60e509b1e066da14461b3832307065726e651153 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 31 Jul 2023 11:09:19 +0100 Subject: [PATCH] Add separate `-vet` flags; `-vet-using-*` flags; `//+vet` file flags --- examples/demo/demo.odin | 1 + src/build_settings.cpp | 35 ++++++++++++++++-- src/check_decl.cpp | 2 +- src/check_expr.cpp | 6 ++-- src/check_stmt.cpp | 6 ++++ src/check_type.cpp | 6 ++++ src/checker.cpp | 24 ++++++++----- src/checker.hpp | 7 ++++ src/main.cpp | 57 +++++++++++++++++++++++++---- src/parser.cpp | 80 +++++++++++++++++++++++++++++++++++++++++ src/parser.hpp | 2 ++ 11 files changed, 204 insertions(+), 22 deletions(-) diff --git a/examples/demo/demo.odin b/examples/demo/demo.odin index 7c98ca728..5f1e84bbf 100644 --- a/examples/demo/demo.odin +++ b/examples/demo/demo.odin @@ -1,3 +1,4 @@ +//+vet !using-stmt !using-param package main import "core:fmt" diff --git a/src/build_settings.cpp b/src/build_settings.cpp index 866631f9a..f234ff2ce 100644 --- a/src/build_settings.cpp +++ b/src/build_settings.cpp @@ -216,6 +216,37 @@ enum BuildPath : u8 { BuildPathCOUNT, }; +enum VetFlags : u64 { + VetFlag_NONE = 0, + VetFlag_Unused = 1u<<0, + VetFlag_Shadowing = 1u<<1, + VetFlag_UsingStmt = 1u<<2, + VetFlag_UsingParam = 1u<<3, + + VetFlag_Extra = 1u<<16, + + VetFlag_All = VetFlag_Unused|VetFlag_Shadowing|VetFlag_UsingStmt, // excluding extra + + VetFlag_Using = VetFlag_UsingStmt|VetFlag_UsingParam, +}; + +u64 get_vet_flag_from_name(String const &name) { + if (name == "unused") { + return VetFlag_Unused; + } else if (name == "shadowing") { + return VetFlag_Shadowing; + } else if (name == "using-stmt") { + return VetFlag_UsingStmt; + } else if (name == "using-param") { + return VetFlag_UsingParam; + } else if (name == "extra") { + return VetFlag_Extra; + } + return VetFlag_NONE; +} + + + // This stores the information for the specify architecture of this build struct BuildContext { // Constants @@ -255,6 +286,8 @@ struct BuildContext { String resource_filepath; String pdb_filepath; + u64 vet_flags; + bool has_resource; String link_flags; String extra_linker_flags; @@ -280,8 +313,6 @@ struct BuildContext { bool no_entry_point; bool no_thread_local; bool use_lld; - bool vet; - bool vet_extra; bool cross_compiling; bool different_os; bool keep_object_files; diff --git a/src/check_decl.cpp b/src/check_decl.cpp index 2b2fb867c..3dca7aafa 100644 --- a/src/check_decl.cpp +++ b/src/check_decl.cpp @@ -1636,7 +1636,7 @@ gb_internal bool check_proc_body(CheckerContext *ctx_, Token token, DeclInfo *de } check_close_scope(ctx); - check_scope_usage(ctx->checker, ctx->scope); + check_scope_usage(ctx->checker, ctx->scope, check_vet_flags(ctx)); add_deps_from_child_to_parent(decl); diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 98154f33d..fe389e027 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -3099,7 +3099,7 @@ gb_internal void check_cast(CheckerContext *c, Operand *x, Type *type) { update_untyped_expr_type(c, x->expr, final_type, true); } - if (build_context.vet_extra) { + if (check_vet_flags(c) & VetFlag_Extra) { if (are_types_identical(x->type, type)) { gbString str = type_to_string(type); warning(x->expr, "Unneeded cast to the same type '%s'", str); @@ -3171,7 +3171,7 @@ gb_internal bool check_transmute(CheckerContext *c, Ast *node, Operand *o, Type return false; } - if (build_context.vet_extra) { + if (check_vet_flags(c) & VetFlag_Extra) { if (are_types_identical(o->type, dst_t)) { gbString str = type_to_string(dst_t); warning(o->expr, "Unneeded transmute to the same type '%s'", str); @@ -10028,7 +10028,7 @@ gb_internal ExprKind check_expr_base_internal(CheckerContext *c, Operand *o, Ast Type *type = type_of_expr(ac->expr); check_cast(c, o, type_hint); if (is_type_typed(type) && are_types_identical(type, type_hint)) { - if (build_context.vet_extra) { + if (check_vet_flags(c) & VetFlag_Extra) { error(node, "Redundant 'auto_cast' applied to expression"); } } diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index a15977b7d..2c1ee8331 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -2464,6 +2464,12 @@ gb_internal void check_stmt_internal(CheckerContext *ctx, Ast *node, u32 flags) error(us->token, "Empty 'using' list"); return; } + if (check_vet_flags(ctx) & VetFlag_UsingStmt) { + ERROR_BLOCK(); + error(node, "'using' as a statement is now allowed when '-vet' or '-vet-using' is applied"); + error_line("\t'using' is considered bad practice to use as a statement outside of immediate refactoring\n"); + } + for (Ast *expr : us->list) { expr = unparen_expr(expr); Entity *e = nullptr; diff --git a/src/check_type.cpp b/src/check_type.cpp index a68f83ba9..c52f32f1a 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -1474,6 +1474,12 @@ gb_internal Type *check_get_params(CheckerContext *ctx, Scope *scope, Ast *_para Type *specialization = nullptr; bool is_using = (p->flags&FieldFlag_using) != 0; + if ((build_context.vet_flags & VetFlag_UsingParam) && is_using) { + ERROR_BLOCK(); + error(param, "'using' on a procedure parameter is now allowed when '-vet' or '-vet-using-stmt' is applied"); + error_line("\t'using' is considered bad practice to use as a statement/procedure parameter outside of immediate refactoring\n"); + + } if (type_expr == nullptr) { param_value = handle_parameter_value(ctx, nullptr, &type, default_value, true); diff --git a/src/checker.cpp b/src/checker.cpp index 2a2cb5c42..a6b66f809 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -655,9 +655,9 @@ gb_internal bool check_vet_unused(Checker *c, Entity *e, VettedEntity *ve) { return false; } -gb_internal void check_scope_usage(Checker *c, Scope *scope) { - bool vet_unused = true; - bool vet_shadowing = true; +gb_internal void check_scope_usage(Checker *c, Scope *scope, u64 vet_flags) { + bool vet_unused = (vet_flags & VetFlag_Unused) != 0; + bool vet_shadowing = (vet_flags & (VetFlag_Shadowing|VetFlag_Using)) != 0; Array vetted_entities = {}; array_init(&vetted_entities, heap_allocator()); @@ -691,15 +691,17 @@ gb_internal void check_scope_usage(Checker *c, Scope *scope) { if (ve.kind == VettedEntity_Shadowed_And_Unused) { error(e->token, "'%.*s' declared but not used, possibly shadows declaration at line %d", LIT(name), other->token.pos.line); - } else if (build_context.vet) { + } else if (vet_flags) { switch (ve.kind) { case VettedEntity_Unused: - error(e->token, "'%.*s' declared but not used", LIT(name)); + if (vet_flags & VetFlag_Unused) { + error(e->token, "'%.*s' declared but not used", LIT(name)); + } break; case VettedEntity_Shadowed: - if (e->flags&EntityFlag_Using) { + if ((vet_flags & (VetFlag_Shadowing|VetFlag_Using)) != 0 && e->flags&EntityFlag_Using) { error(e->token, "Declaration of '%.*s' from 'using' shadows declaration at line %d", LIT(name), other->token.pos.line); - } else { + } else if ((vet_flags & (VetFlag_Shadowing)) != 0) { error(e->token, "Declaration of '%.*s' shadows declaration at line %d", LIT(name), other->token.pos.line); } break; @@ -726,7 +728,7 @@ gb_internal void check_scope_usage(Checker *c, Scope *scope) { if (child->flags & (ScopeFlag_Proc|ScopeFlag_Type|ScopeFlag_File)) { // Ignore these } else { - check_scope_usage(c, child); + check_scope_usage(c, child, vet_flags); } } } @@ -5952,7 +5954,11 @@ gb_internal void check_parsed_files(Checker *c) { TIME_SECTION("check scope usage"); for (auto const &entry : c->info.files) { AstFile *f = entry.value; - check_scope_usage(c, f->scope); + u64 vet_flags = build_context.vet_flags; + if (f->vet_flags_set) { + vet_flags = f->vet_flags; + } + check_scope_usage(c, f->scope, vet_flags); } TIME_SECTION("add basic type information"); diff --git a/src/checker.hpp b/src/checker.hpp index b06d0a8f9..12090cbca 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -449,6 +449,13 @@ struct CheckerContext { Ast *assignment_lhs_hint; }; +u64 check_vet_flags(CheckerContext *c) { + if (c->file && c->file->vet_flags_set) { + return c->file->vet_flags; + } + return build_context.vet_flags; +} + struct Checker { Parser * parser; diff --git a/src/main.cpp b/src/main.cpp index db2702b19..1802e2984 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -654,6 +654,10 @@ enum BuildFlagKind { BuildFlag_NoThreadedChecker, BuildFlag_ShowDebugMessages, BuildFlag_Vet, + BuildFlag_VetShadowing, + BuildFlag_VetUnused, + BuildFlag_VetUsingStmt, + BuildFlag_VetUsingParam, BuildFlag_VetExtra, BuildFlag_IgnoreUnknownAttributes, BuildFlag_ExtraLinkerFlags, @@ -830,8 +834,14 @@ gb_internal bool parse_build_flags(Array args) { add_flag(&build_flags, BuildFlag_UseSeparateModules, str_lit("use-separate-modules"), BuildFlagParam_None, Command__does_build); add_flag(&build_flags, BuildFlag_NoThreadedChecker, str_lit("no-threaded-checker"), BuildFlagParam_None, Command__does_check); add_flag(&build_flags, BuildFlag_ShowDebugMessages, str_lit("show-debug-messages"), BuildFlagParam_None, Command_all); + add_flag(&build_flags, BuildFlag_Vet, str_lit("vet"), BuildFlagParam_None, Command__does_check); + add_flag(&build_flags, BuildFlag_VetUnused, str_lit("vet-unused"), BuildFlagParam_None, Command__does_check); + add_flag(&build_flags, BuildFlag_VetShadowing, str_lit("vet-shadowing"), BuildFlagParam_None, Command__does_check); + add_flag(&build_flags, BuildFlag_VetUsingStmt, str_lit("vet-using-stmt"), BuildFlagParam_None, Command__does_check); + add_flag(&build_flags, BuildFlag_VetUsingParam, str_lit("vet-using-param"), BuildFlagParam_None, Command__does_check); add_flag(&build_flags, BuildFlag_VetExtra, str_lit("vet-extra"), BuildFlagParam_None, Command__does_check); + add_flag(&build_flags, BuildFlag_IgnoreUnknownAttributes, str_lit("ignore-unknown-attributes"), BuildFlagParam_None, Command__does_check); add_flag(&build_flags, BuildFlag_ExtraLinkerFlags, str_lit("extra-linker-flags"), BuildFlagParam_String, Command__does_build); add_flag(&build_flags, BuildFlag_ExtraAssemblerFlags, str_lit("extra-assembler-flags"), BuildFlagParam_String, Command__does_build); @@ -1362,13 +1372,23 @@ gb_internal bool parse_build_flags(Array args) { build_context.show_debug_messages = true; break; case BuildFlag_Vet: - build_context.vet = true; + if (build_context.vet_flags & VetFlag_Extra) { + build_context.vet_flags |= VetFlag_All; + } else { + build_context.vet_flags &= ~VetFlag_Extra; + build_context.vet_flags |= VetFlag_All; + } break; - case BuildFlag_VetExtra: { - build_context.vet = true; - build_context.vet_extra = true; + + case BuildFlag_VetUnused: build_context.vet_flags |= VetFlag_Unused; break; + case BuildFlag_VetShadowing: build_context.vet_flags |= VetFlag_Shadowing; break; + case BuildFlag_VetUsingStmt: build_context.vet_flags |= VetFlag_UsingStmt; break; + case BuildFlag_VetUsingParam: build_context.vet_flags |= VetFlag_UsingParam; break; + + case BuildFlag_VetExtra: + build_context.vet_flags = VetFlag_All | VetFlag_Extra; break; - } + case BuildFlag_IgnoreUnknownAttributes: build_context.ignore_unknown_attributes = true; break; @@ -2124,19 +2144,42 @@ gb_internal void print_show_help(String const arg0, String const &command) { print_usage_line(2, "Multithread the semantic checker stage"); print_usage_line(0, ""); #endif + } + if (check) { print_usage_line(1, "-vet"); print_usage_line(2, "Do extra checks on the code"); print_usage_line(2, "Extra checks include:"); - print_usage_line(3, "Variable shadowing within procedures"); - print_usage_line(3, "Unused declarations"); + print_usage_line(2, "-vet-unused"); + print_usage_line(2, "-vet-shadowing"); + print_usage_line(2, "-vet-using-stmt"); + print_usage_line(0, ""); + + print_usage_line(1, "-vet-unused"); + print_usage_line(2, "Checks for unused declarations"); + print_usage_line(0, ""); + + print_usage_line(1, "-vet-shadowing"); + print_usage_line(2, "Checks for variable shadowing within procedures"); + print_usage_line(0, ""); + + print_usage_line(1, "-vet-using-stmt"); + print_usage_line(2, "Checks for the use of 'using' as a statement"); + print_usage_line(2, "'using' is considered bad practice outside of immediate refactoring"); + print_usage_line(0, ""); + + print_usage_line(1, "-vet-using-param"); + print_usage_line(2, "Checks for the use of 'using' on procedure parameters"); + print_usage_line(2, "'using' is considered bad practice outside of immediate refactoring"); print_usage_line(0, ""); print_usage_line(1, "-vet-extra"); print_usage_line(2, "Do even more checks than standard vet on the code"); print_usage_line(2, "To treat the extra warnings as errors, use -warnings-as-errors"); print_usage_line(0, ""); + } + if (check) { print_usage_line(1, "-ignore-unknown-attributes"); print_usage_line(2, "Ignores unknown attributes"); print_usage_line(2, "This can be used with metaprogramming tools"); diff --git a/src/parser.cpp b/src/parser.cpp index b756412ff..b99182189 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -5528,6 +5528,83 @@ gb_internal bool parse_build_tag(Token token_for_pos, String s) { return any_correct; } +gb_internal String vet_tag_get_token(String s, String *out) { + s = string_trim_whitespace(s); + isize n = 0; + while (n < s.len) { + Rune rune = 0; + isize width = utf8_decode(&s[n], s.len-n, &rune); + if (n == 0 && rune == '!') { + + } else if (!rune_is_letter(rune) && !rune_is_digit(rune) && rune != '-') { + isize k = gb_max(gb_max(n, width), 1); + *out = substring(s, k, s.len); + return substring(s, 0, k); + } + n += width; + } + out->len = 0; + return s; +} + + +gb_internal u64 parse_vet_tag(Token token_for_pos, String s) { + String const prefix = str_lit("+vet"); + GB_ASSERT(string_starts_with(s, prefix)); + s = string_trim_whitespace(substring(s, prefix.len, s.len)); + + if (s.len == 0) { + return VetFlag_All; + } + + + u64 vet_flags = 0; + u64 vet_not_flags = 0; + + while (s.len > 0) { + String p = string_trim_whitespace(vet_tag_get_token(s, &s)); + if (p.len == 0) break; + + bool is_notted = false; + if (p[0] == '!') { + is_notted = true; + p = substring(p, 1, p.len); + if (p.len == 0) { + syntax_error(token_for_pos, "Expected a vet flag name after '!'"); + break; + } + } + + if (p.len == 0) { + continue; + } + + u64 flag = get_vet_flag_from_name(p); + if (flag != VetFlag_NONE) { + if (is_notted) { + vet_not_flags |= flag; + } else { + vet_flags |= flag; + } + } else { + ERROR_BLOCK(); + syntax_error(token_for_pos, "Invalid vet flag name: %.*s", LIT(p)); + error_line("\tExpected one of the following\n"); + error_line("\tunused\n"); + error_line("\tshadowing\n"); + error_line("\tusing-stmt\n"); + error_line("\tusing-param\n"); + error_line("\textra\n"); + break; + } + } + + if (vet_flags == 0 && vet_not_flags != 0) { + vet_flags = VetFlag_All; + } + return vet_flags &~ vet_not_flags; +} + gb_internal String dir_from_path(String path) { String base_dir = path; for (isize i = path.len-1; i >= 0; i--) { @@ -5679,6 +5756,9 @@ gb_internal bool parse_file(Parser *p, AstFile *f) { if (!parse_build_tag(tok, lc)) { return false; } + } else if (string_starts_with(lc, str_lit("+vet"))) { + f->vet_flags = parse_vet_tag(tok, lc); + f->vet_flags_set = true; } else if (string_starts_with(lc, str_lit("+ignore"))) { return false; } else if (string_starts_with(lc, str_lit("+private"))) { diff --git a/src/parser.hpp b/src/parser.hpp index 900fddbab..fa169d3ad 100644 --- a/src/parser.hpp +++ b/src/parser.hpp @@ -104,6 +104,8 @@ struct AstFile { Token package_token; String package_name; + u64 vet_flags; + bool vet_flags_set; // >= 0: In Expression // < 0: In Control Clause