diff --git a/src/build_settings.cpp b/src/build_settings.cpp index 714b8af8f..429540171 100644 --- a/src/build_settings.cpp +++ b/src/build_settings.cpp @@ -2146,9 +2146,8 @@ gb_internal void init_build_context(TargetMetrics *cross_target, Subtarget subta gb_internal bool check_single_target_feature_is_valid(String const &feature_list, String const &feature) { String_Iterator it = {feature_list, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { if (str == feature) { return true; } @@ -2160,8 +2159,8 @@ gb_internal bool check_single_target_feature_is_valid(String const &feature_list gb_internal bool check_target_feature_is_valid(String const &feature, TargetArchKind arch, String *invalid) { String feature_list = target_features_list[arch]; String_Iterator it = {feature, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { String feature_str = str; if (string_starts_with(feature_str, '+') || string_starts_with(feature_str, '-')) { feature_str = substring(feature_str, 1, feature_str.len); @@ -2169,7 +2168,6 @@ gb_internal bool check_target_feature_is_valid(String const &feature, TargetArch return false; } } - if (feature_str == "") break; if (!check_single_target_feature_is_valid(feature_list, feature_str)) { if (invalid) *invalid = str; return false; @@ -2181,10 +2179,8 @@ gb_internal bool check_target_feature_is_valid(String const &feature, TargetArch gb_internal bool check_target_feature_is_valid_globally(String const &feature, String *invalid) { String_Iterator it = {feature, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; - + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { bool valid = false; for (int arch = TargetArch_Invalid; arch < TargetArch_COUNT; arch += 1) { if (check_target_feature_is_valid(str, cast(TargetArchKind)arch, invalid)) { @@ -2208,15 +2204,19 @@ gb_internal bool check_target_feature_is_valid_for_target_arch(String const &fea gb_internal bool check_target_feature_is_enabled(String const &feature, String *not_enabled) { String_Iterator it = {feature, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { String feature_str = str; bool want_enabled = true; if (string_starts_with(feature_str, '+') || string_starts_with(feature_str, '-')) { want_enabled = feature_str[0] == '+'; feature_str = substring(feature_str, 1, feature_str.len); } - if (feature_str == "") break; + if (feature_str == "") { + // a bare sign names no feature, which cannot be enabled + if (not_enabled) *not_enabled = str; + return false; + } String plus_str = concatenate_strings(temporary_allocator(), make_string_c("+"), feature_str); String minus_str = concatenate_strings(temporary_allocator(), make_string_c("-"), feature_str); @@ -2240,9 +2240,8 @@ gb_internal bool check_target_feature_is_enabled(String const &feature, String * gb_internal bool check_target_feature_is_superset_of(String const &superset, String const &of, String *missing) { String_Iterator it = {of, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { if (!check_single_target_feature_is_valid(superset, str)) { if (missing) *missing = str; return false; diff --git a/src/llvm_backend.cpp b/src/llvm_backend.cpp index e770ed90d..6ff3d182a 100644 --- a/src/llvm_backend.cpp +++ b/src/llvm_backend.cpp @@ -3164,10 +3164,9 @@ gb_internal bool lb_generate_code(lbGenerator *gen) { gbString llvm_features = gb_string_make(temporary_allocator(), ""); String_Iterator it = {build_context.target_features_string, 0}; + String str = {}; bool first = true; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; + while (string_split_iterator_next(&it, ',', &str)) { if (!first) { llvm_features = gb_string_appendc(llvm_features, ","); } diff --git a/src/llvm_backend_proc.cpp b/src/llvm_backend_proc.cpp index 42c2f8653..f7e4ebe69 100644 --- a/src/llvm_backend_proc.cpp +++ b/src/llvm_backend_proc.cpp @@ -226,10 +226,9 @@ gb_internal lbProcedure *lb_create_procedure(lbModule *m, Entity *entity, bool i gbString feature_str = gb_string_make(temporary_allocator(), ""); String_Iterator it = {pt->Proc.enable_target_feature, 0}; + String str = {}; bool first = true; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; + while (string_split_iterator_next(&it, ',', &str)) { bool add_prefix = !(string_starts_with(str, '+') || string_starts_with(str, '-')); if (!first) { feature_str = gb_string_appendc(feature_str, ","); diff --git a/src/main.cpp b/src/main.cpp index 84ec22bdf..8785be9c0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1455,12 +1455,8 @@ gb_internal bool parse_build_flags(Array args) { GB_ASSERT(value.kind == ExactValue_String); String val = value.value_string; String_Iterator it = {val, 0}; - for (;;) { - String pkg = string_split_iterator(&it, ','); - if (pkg.len == 0) { - break; - } - + String pkg = {}; + while (string_split_iterator_next(&it, ',', &pkg)) { pkg = string_trim_whitespace(pkg); if (!string_is_valid_identifier(pkg)) { gb_printf_err("-%.*s '%.*s' must be a valid identifier\n", LIT(name), LIT(pkg)); @@ -1478,12 +1474,8 @@ gb_internal bool parse_build_flags(Array args) { GB_ASSERT(value.kind == ExactValue_String); String val = value.value_string; String_Iterator it = {val, 0}; - for (;;) { - String attr = string_split_iterator(&it, ','); - if (attr.len == 0) { - break; - } - + String attr = {}; + while (string_split_iterator_next(&it, ',', &attr)) { attr = string_trim_whitespace(attr); if (!string_is_valid_identifier(attr)) { gb_printf_err("-%.*s '%.*s' must be a valid identifier\n", LIT(name), LIT(attr)); @@ -4166,9 +4158,8 @@ int main(int arg_count, char const **arg_ptr) { } else { String march_list = target_microarch_list[build_context.metrics.arch]; String_Iterator it = {march_list, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { if (str == build_context.microarch) { // Found matching microarch print_microarch_list = false; @@ -4193,9 +4184,8 @@ int main(int arg_count, char const **arg_ptr) { String march_list = target_microarch_list[build_context.metrics.arch]; String_Iterator it = {march_list, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { if (str == default_march) { gb_printf("\t%.*s (default)\n", LIT(str)); } else { @@ -4209,9 +4199,8 @@ int main(int arg_count, char const **arg_ptr) { String default_features = get_default_features(); { String_Iterator it = {default_features, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { string_set_add(&build_context.target_features_set, str); } } @@ -4231,10 +4220,8 @@ int main(int arg_count, char const **arg_ptr) { if (build_context.target_features_string.len != 0) { String_Iterator target_it = {build_context.target_features_string, 0}; - for (;;) { - String item = string_split_iterator(&target_it, ','); - if (item == "") break; - + String item = {}; + while (string_split_iterator_next(&target_it, ',', &item)) { String stripped_item = item; if (*stripped_item.text == '+' || *stripped_item.text == '-') { stripped_item.text++; @@ -4251,9 +4238,8 @@ int main(int arg_count, char const **arg_ptr) { String feature_list = target_features_list[build_context.metrics.arch]; String_Iterator it = {feature_list, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { if (check_single_target_feature_is_valid(default_features, str)) { if (has_ansi_terminal_colours()) { gb_printf("\t%.*s\x1b[38;5;244m (implied by target microarch %.*s)\x1b[0m\n", LIT(str), LIT(march)); diff --git a/src/string.cpp b/src/string.cpp index 76d03d55f..97588a5df 100644 --- a/src/string.cpp +++ b/src/string.cpp @@ -293,6 +293,20 @@ gb_internal String string_split_iterator(String_Iterator *it, const char sep) { return substring(it->str, start, end); } +// NOTE: `string_split_iterator` returns a zero-length `String` both for an empty element and at +// exhaustion, so a loop that stops on an empty result stops at the first empty element instead. +// This skips empty elements and stops only once the iterator is exhausted. +gb_internal bool string_split_iterator_next(String_Iterator *it, char const sep, String *str_) { + while (it->pos < it->str.len) { + String str = string_split_iterator(it, sep); + if (str.len != 0) { + *str_ = str; + return true; + } + } + return false; +} + gb_internal gb_inline bool is_separator(u8 const &ch) { return (ch == '/' || ch == '\\'); } diff --git a/src/types.cpp b/src/types.cpp index d12d8dbee..cf0927cdd 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -3640,9 +3640,8 @@ gb_internal int matched_target_features(TypeProc *t) { int matches = 0; String_Iterator it = {t->require_target_feature, 0}; - for (;;) { - String str = string_split_iterator(&it, ','); - if (str == "") break; + String str = {}; + while (string_split_iterator_next(&it, ',', &str)) { if (check_target_feature_is_valid_for_target_arch(str, nullptr)) { matches += 1; } diff --git a/tests/issues/run.bat b/tests/issues/run.bat index b47988d7c..4cbb424b4 100644 --- a/tests/issues/run.bat +++ b/tests/issues/run.bat @@ -43,13 +43,9 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused ..\..\..\odin test ..\test_issue_7008.odin %COMMON% || exit /b ..\..\..\odin check ..\test_issue_7012.odin -no-entry-point %COMMON% || exit /b ..\..\..\odin check ..\test_issue_7260.odin -no-entry-point %COMMON% || exit /b -..\..\..\odin test ..\test_issue_bool_to_be_conversion.odin %COMMON% || exit /b -..\..\..\odin check ..\test_issue_asm_named_register_slot.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "8" || exit /b ..\..\..\odin check ..\test_issue_ellipsis_type_call.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "10" || exit /b ..\..\..\odin check ..\test_issue_foreign_redeclaration.odin -no-entry-point %COMMON% || exit /b ..\..\..\odin check ..\test_issue_foreign_redeclaration_mismatch.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b -..\..\..\odin check ..\test_issue_asm_rip_register.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "6" || exit /b -..\..\..\odin check ..\test_issue_asm_template_as_value.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "10" || exit /b ..\..\..\odin build ..\test_issue_7037.odin %COMMON% -o:none || exit /b ..\..\..\odin build ..\test_issue_7188.odin %COMMON% || exit /b clang -c ..\test_issue_sysv_abi.c -o test_issue_sysv_abi_c.o || exit /b diff --git a/tests/issues/run.sh b/tests/issues/run.sh index cc7f4deaa..2138e3f0b 100755 --- a/tests/issues/run.sh +++ b/tests/issues/run.sh @@ -97,15 +97,6 @@ $ODIN build ../test_issue_7188.odin $COMMON $ODIN check ../test_issue_7260.odin -no-entry-point $COMMON_CHECK $ODIN test ../test_issue_bool_to_be_conversion.odin $COMMON -# `asm` templates are amd64-only, so this file is empty on every other architecture -if [[ "$(uname -m)" == "x86_64" || "$(uname -m)" == "amd64" ]]; then - if [[ $($ODIN check ../test_issue_asm_named_register_slot.odin -no-entry-point $COMMON_CHECK 2>&1 >/dev/null | grep -c "Error:") -eq 8 ]]; then - echo "SUCCESSFUL 1/1" - else - echo "SUCCESSFUL 0/1" - exit 1 - fi -fi $ODIN check ../test_issue_foreign_redeclaration.odin -no-entry-point $COMMON_CHECK if [[ $($ODIN check ../test_issue_foreign_redeclaration_mismatch.odin -no-entry-point $COMMON_CHECK 2>&1 >/dev/null | grep -c "Error:") -eq 1 ]]; then echo "SUCCESSFUL 1/1" @@ -121,22 +112,6 @@ else exit 1 fi -# `asm` templates are amd64-only, so this file is empty on every other architecture -if [[ "$(uname -m)" == "x86_64" || "$(uname -m)" == "amd64" ]]; then - if [[ $($ODIN check ../test_issue_asm_rip_register.odin -no-entry-point $COMMON_CHECK 2>&1 >/dev/null | grep -c "Error:") -eq 6 ]]; then - echo "SUCCESSFUL 1/1" - else - echo "SUCCESSFUL 0/1" - exit 1 - fi - if [[ $($ODIN check ../test_issue_asm_template_as_value.odin -no-entry-point $COMMON_CHECK 2>&1 >/dev/null | grep -c "Error:") -eq 10 ]]; then - echo "SUCCESSFUL 1/1" - else - echo "SUCCESSFUL 0/1" - exit 1 - fi -fi - if [[ $($ODIN build ../test_issue_7108.odin $COMMON 2>&1 >/dev/null | grep -c "Error:") -eq 2 ]]; then echo "SUCCESSFUL 1/1" else diff --git a/tests/issues/test_issue_asm_named_register_slot.odin b/tests/issues/test_issue_asm_named_register_slot.odin deleted file mode 100644 index 1b6362510..000000000 --- a/tests/issues/test_issue_asm_named_register_slot.odin +++ /dev/null @@ -1,41 +0,0 @@ -#+build amd64 -// A slot that only a named hardware register can fill (segment/control/debug/x87/MMX) -// carries no width and no register class, so it used to absorb any operand at all and -// hand the backend an instruction that does not encode. -package test_issues - -// Rejected: none of these widths pair up, and only `mov`'s segment-register forms ever -// admitted them. -bad_64_32 :: asm(a: i64) -> (r: i32) { mov r, a; } -bad_8_16 :: asm(a: u8) -> (r: u16) { mov r, a; } -bad_16_8 :: asm(a: u16) -> (r: u8) { mov r, a; } -bad_64_8 :: asm(a: u64) -> (r: u8) { mov r, a; } - -// Accepted: equal widths, regardless of signedness or pointer spelling. -ok_32 :: asm(a: i32) -> (r: u32) { mov r, a; } -ok_64 :: asm(a: u64) -> (r: i64) { mov r, a; } -ok_ptr :: asm(a: rawptr) -> (r: ^i32) { mov r, a; } - -// Accepted: the named registers those forms are actually for. -ok_seg :: asm(a: u64) -> (r: u64) { mov %ds, a; mov r, a; } -ok_ctrl :: asm(a: u64) -> (r: u64) { mov %cr0, a; mov r, a; } -ok_dbg :: asm(a: u64) -> (r: u64) { mov %dr0, a; mov r, a; } - -use :: proc() { - a8: u8 - a16: u16 - a32: i32 - a64: i64 - au64: u64 - ap: rawptr - _ = bad_64_32(a64) - _ = bad_8_16(a8) - _ = bad_16_8(a16) - _ = bad_64_8(au64) - _ = ok_32(a32) - _ = ok_64(au64) - _ = ok_ptr(ap) - _ = ok_seg(au64) - _ = ok_ctrl(au64) - _ = ok_dbg(au64) -} diff --git a/tests/issues/test_issue_asm_rip_register.odin b/tests/issues/test_issue_asm_rip_register.odin deleted file mode 100644 index 8b9784790..000000000 --- a/tests/issues/test_issue_asm_rip_register.odin +++ /dev/null @@ -1,15 +0,0 @@ -#+build amd64 -// `%rip` is in the amd64 register table, but its class carries no width, so the checker's width -// switch reached its `GB_PANIC` default arm and aborted with SIGILL instead of diagnosing. Every -// position that can name a register reached it, including `[%rip + disp]`. -package test_issues - -rip_src :: asm() -> (v: u64) { mov v, %rip; } -rip_dst :: asm(x: u64) { mov %rip, x; } -rip_mem :: asm() { mov %rax, [%rip + 8]; } -rip_clob :: asm() [#clobber %rip] { nop; } -rip_in :: asm(x: u64) [x = %rip] { nop; } -rip_out :: asm() -> (r: u64) [r = %rip] { nop; } - -// the nearest special-purpose register that does carry a class has to keep checking cleanly -rsp_ok :: asm() -> (v: u64) { mov v, %rsp; } diff --git a/tests/issues/test_issue_asm_template_as_value.odin b/tests/issues/test_issue_asm_template_as_value.odin deleted file mode 100644 index 2062fa3c7..000000000 --- a/tests/issues/test_issue_asm_template_as_value.odin +++ /dev/null @@ -1,48 +0,0 @@ -#+build amd64 -// A named asm template got a plain `Addressing_Value`, so every value gate let it through: -// a cast, a transmute, an `auto_cast`, a blank assignment, a polymorphic parameter and a -// comparison against `nil` all passed the checker and then aborted the compiler in the -// backend, which has no value to lower for a template. Only a direct call and a listing in -// an `asm` group are legal. -package test_issues - -t :: asm(a: i32) -> (v: i32) { mov v, a; } - -a32 :: asm(a: i32) -> (v: i32) { mov v, a; } -a64 :: asm(a: i64) -> (v: i64) { mov v, a; } -g :: asm { a32, a64 } - -G := cast(rawptr)(t) - -take_rawptr :: proc(p: rawptr) { - _ = p -} - -poly :: proc(x: $T) { - _ = size_of(T) -} - -bad :: proc() { - _ = cast(proc "c" (i32) -> i32)(t) - _ = transmute(proc "c" (i32) -> i32)(t) - _ = cast(rawptr)(t) - _ = transmute(uintptr)(t) - take_rawptr(auto_cast t) - take_rawptr(cast(rawptr)(t)) - _ = t - poly(t) - if t == nil { - take_rawptr(nil) - } -} - -// these forms must remain valid -good :: proc() -> i32 { - x := t(1) - y := (t)(2) - z := g(i32(3)) - w := g(i64(4)) - v := asm(a: i32) -> (v: i32) { mov v, a; }(5) - take_rawptr(G) - return x + y + z + i32(w) + v -}