From 7752cb8ec1278024b8e5c50267586d07bf5efd10 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 8 Aug 2026 08:57:46 -0700 Subject: [PATCH] Reject duplicate class member names involving properties The LRM requires an identifier to name only one item within a scope. Class properties are currently stored only in `class_type_t::properties`, outside the common local symbol table. As a result, declarations such as: class C; int value; function int value; return 0; endfunction endclass are accepted. A second property in a separate declaration silently replaces the first property map entry. A repeated name in a comma-separated declaration such as `int value, value` can crash because both declarators share the same raw type. Make `prop_info_t` a named item and register each property in the class local symbol table. Check the declaration before taking ownership of its type or initializer, since comma-separated properties share the raw declaration type. Use `emplace()` for the property map so an accepted property cannot replace an existing entry. Properties now take part in the common symbol lookup. Give them a distinct symbol type so duplicate declaration diagnostics identify them as class properties, and remove the separate property check from type-identifier classification. Signed-off-by: Lars-Peter Clausen --- PNamedItem.cc | 3 +++ PNamedItem.h | 2 +- pform.cc | 25 ++++++++++++------------- pform.h | 3 +++ pform_pclass.cc | 30 ++++++++++++++++++++++++++---- pform_types.h | 3 ++- 6 files changed, 47 insertions(+), 19 deletions(-) diff --git a/PNamedItem.cc b/PNamedItem.cc index 70c33a7ca..95d9d789c 100644 --- a/PNamedItem.cc +++ b/PNamedItem.cc @@ -48,6 +48,9 @@ std::ostream& operator << (std::ostream&o, PNamedItem::SymbolType st) case PNamedItem::VAR: o << "a variable"; break; + case PNamedItem::CLASS_PROPERTY: + o << "a class property"; + break; case PNamedItem::GENVAR: o << "a genvar"; break; diff --git a/PNamedItem.h b/PNamedItem.h index ba8f86e8f..7e85336e0 100644 --- a/PNamedItem.h +++ b/PNamedItem.h @@ -31,7 +31,7 @@ class PNamedItem : virtual public LineInfo { enum SymbolType { ANY, PARAM, NET, VAR, GENVAR, EVENT, TYPE, ENUM, CLASS, FUNCTION, TASK, BLOCK, GENBLOCK, MODPORT, PACKAGE, MODULE, PROGRAM, INTERFACE, PRIMITIVE, - INSTANCE }; + INSTANCE, CLASS_PROPERTY }; explicit PNamedItem(); virtual ~PNamedItem() override; diff --git a/pform.cc b/pform.cc index c04557c0f..c95f25d55 100644 --- a/pform.cc +++ b/pform.cc @@ -453,7 +453,8 @@ static PGenerate* current_generate_scope() return lexical_scope == pform_cur_generate ? pform_cur_generate : nullptr; } -static void add_local_symbol(LexicalScope*scope, perm_string name, PNamedItem*item) +bool pform_check_local_symbol(LexicalScope *scope, perm_string name, + const PNamedItem *item) { assert(scope); @@ -468,7 +469,7 @@ static void add_local_symbol(LexicalScope*scope, perm_string name, PNamedItem*it "It was declared here as " << cur_sym->second->symbol_type() << "." << endl; error_count += 1; - return; + return false; } // Check for conflict with an explicit import. @@ -479,9 +480,17 @@ static void add_local_symbol(LexicalScope*scope, perm_string name, PNamedItem*it "imported into this scope from package '" << cur_pkg->second.package->pscope_name() << "'." << endl; error_count += 1; - return; + return false; } + return true; +} + +static void add_local_symbol(LexicalScope*scope, perm_string name, PNamedItem*item) +{ + if (!pform_check_local_symbol(scope, name, item)) + return; + scope->local_symbols[name] = item; } @@ -951,16 +960,6 @@ typedef_t* pform_test_type_identifier(const struct vlltype&loc, const char*txt) if (sym != cur_scope->local_symbols.end()) return nullptr; - // Class properties are tracked in the class type, not in - // local_symbols, but still shadow type names before lookup falls - // through to wildcard imports. - if (auto cur_class = dynamic_cast (cur_scope)) { - if (cur_class->type && - cur_class->type->properties.find(name) != - cur_class->type->properties.end()) - return nullptr; - } - PPackage*pkg = pform_find_potential_import(loc, cur_scope, name, false, false); if (pkg) { cur = pkg->typedefs.find(name); diff --git a/pform.h b/pform.h index fb1e20d9b..46b4f1392 100644 --- a/pform.h +++ b/pform.h @@ -259,6 +259,9 @@ extern void pform_pop_block_scope(bool keep_scope); */ extern LexicalScope* pform_peek_scope(); +extern bool pform_check_local_symbol(LexicalScope *scope, perm_string name, + const PNamedItem *item); + extern PClass* pform_push_class_scope(const struct vlltype&loc, perm_string name); extern PFunction*pform_push_constructor_scope(const struct vlltype&loc); diff --git a/pform_pclass.cc b/pform_pclass.cc index 2e6694677..cb9d3dfb6 100644 --- a/pform_pclass.cc +++ b/pform_pclass.cc @@ -18,6 +18,7 @@ */ # include +# include # include "pform.h" # include "PClass.h" # include "parse_misc.h" @@ -78,6 +79,30 @@ void pform_class_property(const struct vlltype&loc, ; cur != decls->end() ; ++cur) { decl_assignment_t*curp = *cur; + class_type_t::prop_info_t property(property_qual, nullptr, + curp->expr != nullptr); + FILE_NAME(&property, loc); + + // Properties are stored by value in the property map, while the type + // can be shared by comma-separated declarations. Check the temporary + // before taking ownership of the type or releasing the initializer. It + // can not be registered in the local symbol table yet since that would + // store a pointer to the temporary. + if (!pform_check_local_symbol(pform_cur_class, curp->name.first, + &property)) + continue; + + auto result = pform_cur_class->type->properties.emplace( + curp->name.first, std::move(property)); + ivl_assert(loc, result.second); + + auto &property_info = result.first->second; + // Register the property now that it has a stable address. The earlier + // check makes failure of this insertion an internal error. + auto symbol = pform_cur_class->local_symbols.emplace( + curp->name.first, &property_info); + ivl_assert(loc, symbol.second); + data_type_t*use_type = data_type; if (! curp->index.empty()) { @@ -86,10 +111,7 @@ void pform_class_property(const struct vlltype&loc, FILE_NAME(use_type, loc); } - pform_cur_class->type->properties[curp->name.first] - = class_type_t::prop_info_t(property_qual, use_type, - curp->expr != nullptr); - FILE_NAME(&pform_cur_class->type->properties[curp->name.first], loc); + property_info.type.reset(use_type); if (PExpr*rval = curp->expr.release()) { PExpr*lval = new PEIdent(curp->name.first, curp->name.second); diff --git a/pform_types.h b/pform_types.h index f18e12608..ae8857ccc 100644 --- a/pform_types.h +++ b/pform_types.h @@ -408,13 +408,14 @@ struct class_type_t : public data_type_t { bool virtual_class; // This is a map of the properties. Map the name to the type. - struct prop_info_t : public LineInfo { + struct prop_info_t : public PNamedItem { inline prop_info_t() : qual(property_qualifier_t::make_none()) { } inline prop_info_t(property_qualifier_t q, data_type_t *t, bool init) : qual(q), type(t), has_initializer(init) { } prop_info_t(prop_info_t&&) = default; prop_info_t& operator=(prop_info_t&&) = default; + SymbolType symbol_type() const override { return CLASS_PROPERTY; } property_qualifier_t qual; std::unique_ptr type; bool has_initializer = false;