================ @@ -0,0 +1,48 @@ +//===- EntityIdTable.cpp ----------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Analysis/Scalable/Model/EntityIdTable.h" +#include <algorithm> +#include <cassert> + +namespace clang { +namespace ssaf { + +EntityId EntityIdTable::createEntityId(const EntityName &Name) { ---------------- aviralg wrote:
This API sets the wrong expectation for the user. 1. It is named `createEntityId` suggesting that it is always going to create an EntityId. However it performs an extra function that if the entity already exists, it returns the assigned id. 2. On the client side this kind of API functionality can lead to subtle issues. A client wants to create an ID for a new entity. Due to a bug in their code, the entity is already present in the table. Instead of throwing an error and letting the client know that their expected invariant has been violated, this function returns the existing ID. I propose the following APIs instead: 1. `EntityId EntityTable::create(const EntityName& Name)` that will only insert the Name under the assumption that its new. If the name already exists, API throws an error. 2. `const EntityName& EntityTable::lookup(EntityId Id)` which gives name for an id; error if id does not exist. 3. `EntityId EntityTable::lookup(const EntityName&)` which gives id for a name; error if name does not exist. 4. `bool EntityTable::exists(EntityId Id)` checks if entity with id exists. 5. `bool EntityTable::exists(const EntityName&)` checks if entity with name exists. If we really want an API that either creates an Id or returns the Id if name already exists, it should be called `createOrLookup`. https://github.com/llvm/llvm-project/pull/171660 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
