From 444f4f446a8272ca3a1ab763c92bc18375c4d947 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Sun, 25 Nov 2018 14:14:58 +0000 Subject: [PATCH] `-vet` flag to do basic vetting of code --- core/fmt/fmt.odin | 7 +- core/mem/alloc.odin | 4 +- examples/demo/demo.odin | 1 - src/build_settings.cpp | 1 + src/checker.cpp | 159 +++++++++++++++++++++++++++++++++++----- src/checker.hpp | 1 - src/main.cpp | 6 ++ 7 files changed, 152 insertions(+), 27 deletions(-) diff --git a/core/fmt/fmt.odin b/core/fmt/fmt.odin index eb0b312e8..4de8cb314 100644 --- a/core/fmt/fmt.odin +++ b/core/fmt/fmt.odin @@ -843,7 +843,6 @@ fmt_bit_set :: proc(fi: ^Fmt_Info, v: any, name: string = "") { case runtime.Type_Info_Bit_Set: bits: u64; bit_size := u64(8*type_info.size); - verb := 'b'; switch bit_size { case 0: bits = 0; @@ -886,7 +885,7 @@ fmt_bit_set :: proc(fi: ^Fmt_Info, v: any, name: string = "") { } } } -fmt_bit_field :: proc(fi: ^Fmt_Info, v: any, name: string = "") { +fmt_bit_field :: proc(fi: ^Fmt_Info, v: any, bit_field_name: string = "") { type_info := type_info_of(v.id); switch info in type_info.variant { case runtime.Type_Info_Named: @@ -902,8 +901,8 @@ fmt_bit_field :: proc(fi: ^Fmt_Info, v: any, name: string = "") { case 8: data = cast(u64)(^u64)(v.data)^; } - if name != "" { - write_string(fi.buf, name); + if bit_field_name != "" { + write_string(fi.buf, bit_field_name); write_byte(fi.buf, '{'); } else { write_string(fi.buf, "bit_field{"); diff --git a/core/mem/alloc.odin b/core/mem/alloc.odin index 3cf2766bd..589afb17b 100644 --- a/core/mem/alloc.odin +++ b/core/mem/alloc.odin @@ -226,9 +226,9 @@ scratch_allocator_proc :: proc(allocator_data: rawptr, mode: Allocator_Mode, case Allocator_Mode.Free: last_ptr := rawptr(&scratch.data[scratch.prev_offset]); if old_memory == last_ptr { - size := scratch.curr_offset - scratch.prev_offset; + full_size := scratch.curr_offset - scratch.prev_offset; scratch.curr_offset = scratch.prev_offset; - zero(last_ptr, size); + zero(last_ptr, full_size); return nil; } // NOTE(bill): It's scratch memory, don't worry about freeing diff --git a/examples/demo/demo.odin b/examples/demo/demo.odin index 7b35bbd2f..1a4b48037 100644 --- a/examples/demo/demo.odin +++ b/examples/demo/demo.odin @@ -408,7 +408,6 @@ parametric_polymorphism :: proc() { } assert(table.count <= len(table.slots)); - hash := get_hash(key); index = int(hash % u32(len(table.slots))); for table.slots[index].occupied { diff --git a/src/build_settings.cpp b/src/build_settings.cpp index 218544aaa..e8c872c0a 100644 --- a/src/build_settings.cpp +++ b/src/build_settings.cpp @@ -103,6 +103,7 @@ struct BuildContext { bool no_output_files; bool no_crt; bool use_lld; + bool vet; gbAffinity affinity; isize thread_count; diff --git a/src/checker.cpp b/src/checker.cpp index d289d236c..75ff5ea65 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -320,6 +320,8 @@ void check_open_scope(CheckerContext *c, Ast *node) { case Ast_StructType: case Ast_EnumType: case Ast_UnionType: + case Ast_BitFieldType: + case Ast_BitSetType: scope->flags |= ScopeFlag_Type; break; } @@ -413,41 +415,160 @@ GB_COMPARE_PROC(entity_variable_pos_cmp) { } +enum VettedEntityKind { + VettedEntity_Invalid, + + VettedEntity_Unused, + VettedEntity_Shadowed, +}; +struct VettedEntity { + VettedEntityKind kind; + Entity *entity; + Entity *other; +}; +void init_vetted_entity(VettedEntity *ve, VettedEntityKind kind, Entity *entity, Entity *other=nullptr) { + ve->kind = kind; + ve->entity = entity; + ve->other = other; +} + + +GB_COMPARE_PROC(vetted_entity_variable_pos_cmp) { + Entity *x = (*cast(VettedEntity **)a)->entity; + Entity *y = (*cast(VettedEntity **)b)->entity; + + return token_pos_cmp(x->token.pos, y->token.pos); +} + + + +bool check_vet_shadowing(Checker *c, Entity *e, VettedEntity *ve) { + if (e->kind != Entity_Variable) { + return false; + } + String name = e->token.string; + if (name == "_") { + return false; + } + if (e->flags & EntityFlag_Param) { + return false; + } + + if (e->scope->flags & (ScopeFlag_Global|ScopeFlag_File|ScopeFlag_Proc)) { + return false; + } + + Scope *parent = e->scope->parent; + if (parent->flags & (ScopeFlag_Global|ScopeFlag_File)) { + return false; + } + + Entity *shadowed = scope_lookup(parent, name); + if (shadowed == nullptr) { + return false; + } + if (shadowed->kind != Entity_Variable) { + return false; + } + + if (shadowed->scope->flags & (ScopeFlag_Global|ScopeFlag_File)) { + // return false; + } + + // NOTE(bill): The entities must be in the same file + if (e->token.pos.file != shadowed->token.pos.file) { + return false; + } + // NOTE(bill): The shaded identifier must appear before this one to be an + // instance of shadowing + if (token_pos_cmp(shadowed->token.pos, e->token.pos) > 0) { + return false; + } + // NOTE(bill): If the types differ, don't complain + if (are_types_identical(e->type, shadowed->type)) { + gb_zero_item(ve); + ve->kind = VettedEntity_Shadowed; + ve->entity = e; + ve->other = shadowed; + return true; + } + + return false; +} + +bool check_vet_unused(Checker *c, Entity *e, VettedEntity *ve) { + if ((e->flags&EntityFlag_Used) == 0) { + switch (e->kind) { + case Entity_Variable: + case Entity_ImportName: + case Entity_LibraryName: + gb_zero_item(ve); + ve->kind = VettedEntity_Unused; + ve->entity = e; + return true; + } + } + return false; +} + void check_scope_usage(Checker *c, Scope *scope) { - // TODO(bill): Use this? -#if 0 - Array unused = {}; - array_init(&unused, heap_allocator()); - defer (array_free(&unused)); + if (!build_context.vet) { + return; + } + + bool vet_unused = true; + bool vet_shadowing = true; + + Array vetted_entities = {}; + array_init(&vetted_entities, heap_allocator()); for_array(i, scope->elements.entries) { Entity *e = scope->elements.entries[i].value; - if (e != nullptr && (e->flags&EntityFlag_Used) == 0) { - switch (e->kind) { - case Entity_Variable: - case Entity_ImportName: - case Entity_LibraryName: - array_add(&unused, e); - break; - } + if (e == nullptr) continue; + VettedEntity ve = {}; + if (vet_unused && check_vet_unused(c, e, &ve)) { + array_add(&vetted_entities, ve); + } + if (vet_shadowing && check_vet_shadowing(c, e, &ve)) { + array_add(&vetted_entities, ve); } } - gb_sort_array(unused.data, unused.count, entity_variable_pos_cmp); + gb_sort_array(vetted_entities.data, vetted_entities.count, vetted_entity_variable_pos_cmp); - for_array(i, unused) { - Entity *e = unused[i]; - error(e->token, "'%.*s' declared but not used", LIT(e->token.string)); + for_array(i, vetted_entities) { + auto ve = vetted_entities[i]; + Entity *e = ve.entity; + Entity *other = ve.other; + String name = e->token.string; + + switch (ve.kind) { + case VettedEntity_Unused: + error(e->token, "'%.*s' declared but not used", LIT(name)); + break; + case VettedEntity_Shadowed: + if (e->flags&EntityFlag_Using) { + error(e->token, "Declaration of '%.*s' from 'using' shadows declaration at line %lld", LIT(name), cast(long long)other->token.pos.line); + } else { + error(e->token, "Declaration of '%.*s' shadows declaration at line %lld", LIT(name), cast(long long)other->token.pos.line); + } + break; + default: + break; + } } + array_free(&vetted_entities); + for (Scope *child = scope->first_child; child != nullptr; child = child->next) { - if (!child->is_proc && !child->is_struct && !child->is_file) { + if (child->flags & (ScopeFlag_Proc|ScopeFlag_Type|ScopeFlag_File)) { + // Ignore these + } else { check_scope_usage(c, child); } } -#endif } diff --git a/src/checker.hpp b/src/checker.hpp index 7b0ad4653..43056d3ff 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -349,7 +349,6 @@ enum ScopeFlag { ScopeFlag_Proc = 1<<5, ScopeFlag_Type = 1<<6, - ScopeFlag_HasBeenImported = 1<<10, // This is only applicable to file scopes }; diff --git a/src/main.cpp b/src/main.cpp index ed72b5f0a..10681cd4d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -214,6 +214,7 @@ enum BuildFlagKind { BuildFlag_NoBoundsCheck, BuildFlag_NoCRT, BuildFlag_UseLLD, + BuildFlag_Vet, BuildFlag_COUNT, }; @@ -256,6 +257,7 @@ bool parse_build_flags(Array args) { add_flag(&build_flags, BuildFlag_NoBoundsCheck, str_lit("no-bounds-check"), BuildFlagParam_None); add_flag(&build_flags, BuildFlag_NoCRT, str_lit("no-crt"), BuildFlagParam_None); add_flag(&build_flags, BuildFlag_UseLLD, str_lit("lld"), BuildFlagParam_None); + add_flag(&build_flags, BuildFlag_Vet, str_lit("vet"), BuildFlagParam_None); GB_ASSERT(args.count >= 3); Array flag_args = array_slice(args, 3, args.count); @@ -563,6 +565,10 @@ bool parse_build_flags(Array args) { case BuildFlag_UseLLD: build_context.use_lld = true; break; + + case BuildFlag_Vet: + build_context.vet = true; + break; } }