Issue warnings on some potential name conflicts

Author: mikkelfj

CHANGELOG.md

@@ -63,6 +63,8 @@
 - Fix GCC 15 warning in grisu3 (#338).
 - Bump CMake to >= 3.26.1 (Debian Bookworm).
 - Fix CMAKE warnings on custom commands with TARGET and DEPENDS (CMP0175).
+- Issue warnings on some potential name conflicts that can break C source
+  and add `-s` option to silence warnings (#354).
 
 ## [0.6.1]
 

README.md

@@ -330,6 +330,10 @@ that for clang debug builds, -fsanitize=undefined has been added and this may
 require dependent source code to also use that flag to avoid missing linker
 symbols. The feature can be disabled in CMakeLists.txt. CMake has been bumped to
 version 3.16 which is the latest version where Appveyor will build MVSC 2015.
+Warnings added for potential name conflicts in enum names and table fields
+that could result in confusing C compiler errors. Not all warnigs are errors
+to the option `-s` was also added to silence warnings. The existing `-g`
+option can resolve some conflicts.
 
 Release 0.6.1 contains primarily bug fixes and numerous contributions from the
 community to handle platform edge cases. Additionally, pendantic GCC warnings
@@ -1208,6 +1212,10 @@ If unfortunate, it is possible to have a read accessor method conflict
 with other generated methods and typenames. Usually a small change in
 the schema will resolve this issue.
 
+As of flatcc 0.6.2 conservative warnings will detect most likely
+potential conflicts in enum member names and table field names. The
+`-s` option silences warnings.
+
 As of flatcc 0.5.2 read accors are generated with and without a `_get`
 suffix so it is also possible to use `Monster_pos_get(monster)` instead
 of `Monster_pos(monster)`. When calling flatcc with option `-g` the

include/flatcc/flatcc.h

@@ -54,6 +54,7 @@ struct flatcc_options {
     int bool_size;
     int require_root_type;
     int strict_enum_init;
+    int silence;
     uint64_t vt_max_count;
 
     const char *default_schema_ext;

src/cli/flatcc_cli.c

@@ -23,6 +23,7 @@ void usage(FILE *fp)
             "  -r, --recursive            Recursively generate included schema files\n"
             "  -a                         Generate all (like -cwvr)\n"
             "  -g                         Use _get suffix only to avoid conflicts\n"
+            "  -s                         Silence warnings\n"
             "  -d                         Dependency file like gcc -MMD\n"
             "  -I<inpath>                 Search path for include files (multiple allowed)\n"
             "  -o<outpath>                Write files relative to this path (dir must exist)\n"
@@ -83,6 +84,11 @@ void help(FILE *fp)
         "'Monster_name(monster)'. This avoids potential conflicts with\n"
         "other generated symbols when a schema change is impractical.\n"
         "\n"
+        "-s Silence warnings, notably on potential conflicts that might cause\n"
+        "C source code to generate errors. These warnings are not perfect and can\n"
+        "be ignored if the C code compiles, though it is recommended to change\n"
+        "names in the schema if possible.\n"
+        "\n"
         "-d generates a dependency file, e.g. 'monster.fbs.d' in the output dir.\n"
         "\n"
         "--depfile implies -d but accepts an explicit filename with a path\n"
@@ -344,6 +350,9 @@ int set_opt(flatcc_options_t *opts, const char *s, const char *a)
     case 'g':
         opts->cgen_no_conflicts = 1;
         return noarg;
+    case 's':
+        opts->silence = 1;
+        return noarg;
     case 'd':
         opts->gen_dep = 1;
         return noarg;

src/compiler/parser.c

@@ -58,10 +58,14 @@ const char *error_find_file_of_token(fb_parser_t *P, fb_token_t *t)
     return "";
 }
 
-void error_report(fb_parser_t *P, fb_token_t *t, const char *msg, fb_token_t *peer, const char *s, size_t len)
+void error_report_ext(fb_parser_t *P, fb_token_t *t, const char *msg, fb_token_t *peer, const char *s, size_t len, int is_warning)
 {
     const char *file, *peer_file;
+    const char *kind = is_warning ? "warning" : "error";
 
+    if (is_warning && P->opts.silence) {
+        return;
+    }
     if (t && !s) {
         s = t->text;
         len = (size_t)t->len;
@@ -75,25 +79,37 @@ void error_report(fb_parser_t *P, fb_token_t *t, const char *msg, fb_token_t *pe
     }
     if (t && !peer) {
         file = error_find_file_of_token(P, t);
-        fb_print_error(P, "%s:%ld:%ld: error: '%.*s': %s\n",
-                file, (long)t->linenum, (long)t->pos, len, s, msg);
+        fb_print_error(P, "%s:%ld:%ld: %s: '%.*s': %s\n",
+                file, (long)t->linenum, (long)t->pos, kind, len, s, msg);
     } else if (t && peer) {
         file = error_find_file_of_token(P, t);
         peer_file = error_find_file_of_token(P, peer);
-        fb_print_error(P, "%s:%ld:%ld: error: '%.*s': %s: %s:%ld:%ld: '%.*s'\n",
-                file, (long)t->linenum, (long)t->pos, len, s, msg,
+        fb_print_error(P, "%s:%ld:%ld: %s: '%.*s': %s: %s:%ld:%ld: '%.*s'\n",
+                file, (long)t->linenum, (long)t->pos, kind, len, s, msg,
                 peer_file, (long)peer->linenum, (long)peer->pos, (int)peer->len, peer->text);
     } else if (!t && !peer) {
         fb_print_error(P, "error: %s\n", msg);
     } else if (peer) {
         peer_file = error_find_file_of_token(P, peer);
-        fb_print_error(P, "error: %s: %s:%ld:%ld: '%.*s'\n",
-                msg,
+        fb_print_error(P, "%s: %s: %s:%ld:%ld: '%.*s'\n",
+                kind, msg,
                 peer_file, (long)peer->linenum, (long)peer->pos, (int)peer->len, peer->text);
     } else {
         fb_print_error(P, "internal error: unexpected state\n");
     }
-    ++P->failed;
+    if (!is_warning) {
+        ++P->failed;
+    }
+}
+
+void error_report(fb_parser_t *P, fb_token_t *t, const char *msg, fb_token_t *peer, const char *s, size_t len)
+{
+    error_report_ext(P, t, msg, peer, s, len, 0);
+}
+
+void warn_report(fb_parser_t *P, fb_token_t *t, const char *msg, fb_token_t *peer, const char *s, size_t len)
+{
+    error_report_ext(P, t, msg, peer, s, len, 1);
 }
 
 void error_ref_sym(fb_parser_t *P, fb_ref_t *ref, const char *msg, fb_symbol_t *s2)

src/compiler/parser.h

@@ -127,6 +127,9 @@ const char *__flatcc_error_find_file_of_token(fb_parser_t *P, fb_token_t *t);
 void __flatcc_error_report(fb_parser_t *P, fb_token_t *t, const char *msg, fb_token_t *peer, const char *s, size_t len);
 #define error_report __flatcc_error_report
 
+void __flatcc_warn_report(fb_parser_t *P, fb_token_t *t, const char *msg, fb_token_t *peer, const char *s, size_t len);
+#define warn_report __flatcc_warn_report
+
 static void error_tok_2(fb_parser_t *P, fb_token_t *t, const char *msg, fb_token_t *peer)
 {
     error_report(P, t, msg, peer, 0, 0);
@@ -137,6 +140,17 @@ static inline void error_tok(fb_parser_t *P, fb_token_t *t, const char *msg)
     error_tok_2(P, t, msg, 0);
 }
 
+static inline void warn_tok(fb_parser_t *P, fb_token_t *t, const char *msg)
+{
+    warn_report(P, t, msg, 0, 0, 0);
+}
+
+/* Show both token and string */
+static inline void warn_tok_as_string(fb_parser_t *P, fb_token_t *t, const char *msg, const char *s, size_t len)
+{
+    warn_report(P, t, msg, 0, s, len);
+}
+
 /* Only use the token location. */
 static inline void error_tok_as_string(fb_parser_t *P, fb_token_t *t, const char *msg, char *s, size_t len)
 {

src/compiler/semantics.c

@@ -29,6 +29,87 @@ static const char *fb_known_attribute_names[] = {
     "sorted",
 };
 
+static const char *fb_reserved_kw_enum_members[] = {
+    "_const_ptr_add",
+    "_ptr_add",
+    "_size",
+    "array_copy",
+    "array_copy_from_pe",
+    "array_copy_to_pe",
+    "assign",
+    "assign_from_pe",
+    "assign_to_pe",
+    "cast_from_be",
+    "cast_from_le",
+    "cast_from_pe",
+    "cast_to_be",
+    "cast_to_le",
+    "cast_to_pe",
+    "copy",
+    "copy_from_pe",
+    "copy_to_pe",
+    "enum_t",
+    "from_pe",
+    "is_known_value",
+    "name",
+    "parse_json_enum",
+    "print_json_enum",
+    "read",
+    "read_from_pe",
+    "read_to_pe",
+    "to_pe",
+    // "vec_*",
+    "write",
+    "write_from_pe",
+    "write_to_pe",
+};
+
+static const char *fb_reserved_kw_table_fields[] = {
+      "add_type",
+      "add_value",
+      "begin",
+      "clone",
+      "create",
+      "end",
+      "end_pe",
+      "force_add",
+      "table_t",
+      "verity_table",
+};
+
+static const char *fb_reserved_kw_table_field_prefixes[] = {
+      "as_",
+      "clone_as_",
+      "create_as_",
+      "end_as_",
+      "parse_json_",
+      "print_json_",
+      "start_as_",
+      // "vec_  ",
+      "verify_as_*",
+};
+
+static const char *fb_reserved_kw_table_field_suffixes[] = {
+      "_add",
+      "_clone",
+      "_create",
+      "_end",
+      "_get",
+      "_get_ptr",
+      "_force_add",
+      "_is_present",
+      "_pick",
+      "_push_start",
+      "_push_end",
+      "_push_create",
+      "_start",
+};
+
+/* For table and enum members. */
+static const char *fb_reserved_kw_vec_prefixes[] = {
+      "vec_",
+};
+
 static const int fb_known_attribute_types[] = {
     vt_invalid, /* Unknowns have arbitrary types. */
     vt_uint,
@@ -396,6 +477,110 @@ static void install_known_attributes(fb_parser_t *P)
     }
 }
 
+static void install_reserved_keyword_table(fb_parser_t *P, const char *table[], size_t count, int kind)
+{
+    fb_reserved_kw_t *kw;
+    size_t i;
+
+    for (i = 0; i < count; ++i) {
+        kw = new_elem(P, sizeof(*kw));
+        kw->name.name.s.s = (char *)table[i];
+        kw->name.name.s.len = (int)strlen(table[i]);
+        kw->name.link = 0;
+        kw->kind = kind;
+        define_fb_name(&P->schema.root_schema->keyword_index, &kw->name);
+    }
+}
+
+#define KW_TABLE_LEN(T) (sizeof(T)/sizeof(T[0]))
+
+static void install_reserved_keywords(fb_parser_t *P)
+{
+    install_reserved_keyword_table(P, fb_reserved_kw_enum_members,
+                                   KW_TABLE_LEN(fb_reserved_kw_enum_members),
+                                   fb_reserved_kw_kind_enum_member);
+    install_reserved_keyword_table(P, fb_reserved_kw_table_fields,
+                                   KW_TABLE_LEN(fb_reserved_kw_table_fields),
+                                   fb_reserved_kw_kind_table_field);
+    install_reserved_keyword_table(P, fb_reserved_kw_table_field_prefixes,
+                                   KW_TABLE_LEN(fb_reserved_kw_table_field_prefixes),
+                                   fb_reserved_kw_kind_table_field_prefix);
+    install_reserved_keyword_table(P, fb_reserved_kw_table_field_suffixes,
+                                   KW_TABLE_LEN(fb_reserved_kw_table_field_suffixes),
+                                   fb_reserved_kw_kind_table_field_suffix);
+    install_reserved_keyword_table(P, fb_reserved_kw_vec_prefixes,
+                                   KW_TABLE_LEN(fb_reserved_kw_vec_prefixes),
+                                   fb_reserved_kw_kind_vec_prefix);
+}
+
+static void check_reserved_enum_member(fb_parser_t *P, fb_symbol_t *sym)
+{
+    const char *text = sym->ident->text;
+    size_t len = (size_t)sym->ident->len;
+    fb_reserved_kw_t *kw = 0;
+    size_t i;
+
+    kw = (fb_reserved_kw_t *)fb_name_table_find(&P->schema.root_schema->keyword_index,
+                                                         text, len);
+    if (kw && kw->kind == fb_reserved_kw_kind_enum_member) {
+        warn_tok(P, sym->ident, "potential enum member name conflict in C source");
+        warn_tok_as_string(P, sym->ident, "name is reserved", kw->name.name.s.s, (size_t)kw->name.name.s.len);
+        return;
+    }
+    for (i = 0; i < len; ++i) {
+        if (text[i] =='_') {
+            if (i + 1 < len) {
+                kw = (fb_reserved_kw_t *)fb_name_table_find(&P->schema.root_schema->keyword_index,
+                                                         text, i + 1);
+                if (kw && (kw->kind == fb_reserved_kw_kind_vec_prefix)) {
+                    warn_tok(P, sym->ident, "potential enum member name conflict in C source");
+                    warn_tok_as_string(P, sym->ident, "prefix is reserved", kw->name.name.s.s, (size_t)kw->name.name.s.len);
+                    return;
+                }
+            }
+        }
+    }
+}
+
+static void check_reserved_table_field(fb_parser_t *P, fb_symbol_t *sym)
+{
+    const char *text = sym->ident->text;
+    size_t len = (size_t)sym->ident->len;
+    fb_reserved_kw_t *kw = 0;
+    size_t i;
+
+    kw = (fb_reserved_kw_t *)fb_name_table_find(&P->schema.root_schema->keyword_index,
+                                                         text, len);
+    if (kw && kw->kind == fb_reserved_kw_kind_table_field) {
+        warn_tok(P, sym->ident, "potential table field conflict in C source (option -g might help)");
+        warn_tok_as_string(P, sym->ident, "name is reserved", kw->name.name.s.s, (size_t)kw->name.name.s.len);
+        return;
+    }
+    for (i = 0; i < len; ++i) {
+        if (text[i] =='_') {
+            if (i + 1 < len) {
+                kw = (fb_reserved_kw_t *)fb_name_table_find(&P->schema.root_schema->keyword_index,
+                                                         text, i + 1);
+                if (kw && (kw->kind == fb_reserved_kw_kind_table_field_prefix
+                    || kw->kind == fb_reserved_kw_kind_vec_prefix)) {
+                    warn_tok(P, sym->ident, "potential table field conflict in C source (option -g might help)");
+                    warn_tok_as_string(P, sym->ident, "prefix is reserved", kw->name.name.s.s, (size_t)kw->name.name.s.len);
+                    return;
+                }
+            }
+            if (i > 0) {
+                kw = (fb_reserved_kw_t *)fb_name_table_find(&P->schema.root_schema->keyword_index,
+                                                         text + i, len - i);
+                if (kw && kw->kind == fb_reserved_kw_kind_table_field_suffix) {
+                    warn_tok(P, sym->ident, "potential table field conflict in C source (option -g might help)");
+                    warn_tok_as_string(P, sym->ident, "suffix is reserved", kw->name.name.s.s, (size_t)kw->name.name.s.len);
+                    return;
+                }
+            }
+        }
+    }
+}
+
 static void revert_order(fb_compound_type_t **list) {
     fb_compound_type_t *next, *prev = 0, *link = *list;
 
@@ -948,6 +1133,7 @@ static int process_table(fb_parser_t *P, fb_compound_type_t *ct)
             error_sym(P, sym, "internal error: member type expected");
             return -1;
         }
+        check_reserved_table_field(P, sym);
         member = (fb_member_t *)sym;
         if (member->type.type == vt_invalid) {
             continue;
@@ -1619,6 +1805,9 @@ static int process_enum(fb_parser_t *P, fb_compound_type_t *ct)
 
     for (sym = ct->members; sym; sym = sym->link, first = 0) {
         member = (fb_member_t *)sym;
+        if (!is_union) {
+            check_reserved_enum_member(P, sym);
+        }
         if ((old = define_fb_symbol(&ct->index, sym))) {
             if (old->ident == &P->t_none) {
                 /*
@@ -1822,6 +2011,7 @@ int fb_build_schema(fb_parser_t *P)
         }
     }
     install_known_attributes(P);
+    install_reserved_keywords(P);
 
     if (!P->opts.hide_later_enum) {
         for (sym = S->symbols; sym; sym = sym->link) {