[PATCH] D89987: [analyzer] [NFC] Rename SymbolRef to SymExprRef

Denys Petrov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 23 06:35:03 PDT 2020


ASDenysPetrov added a comment.

@OikawaKirie

> Different from ProgramStateRef which is an alias to IntrusiveRefCntPtr, or StoreRef which is a wrapper object, an alias to a const SymExpr * makes no sense to me.

Yes. I omit this, because in such case we should go further and rename all those which are not real `Ref` to `Ptr` or smth that would emphasise that it's just a pointer alias, not a wrapper or another class.
That's why I prefered to change the name a little in favor of complex approach of renaming all the rest.

> And this is also where I have been confused for a long while.

So have been I. The patch is called to make it more clear :)

Thanks to @steakhal comment I investigated more in terms of what other names use `Symbol` but mean `SymExpr`. They are:

  class SymbolManager;
  using SymbolID;
  using SymbolDependTy;
  class SymbolData;
  class SymbolMetadata;
  class SymbolReaper;
  enum SymbolStatus;
  using SymbolSetTy;
  using SymbolMapTy;
  class SymbolCast;
  class SymbolVal;
  class symbol_iterator;
  etc.

This is not a full list! I also didn't count //methods// and //file names//.
There is also a list of objects names which straightly use `SymExpr`. They are less spread. Mostly they a derived classes:

  class BinarySymExprImpl;
  class SymIntExpr;
  class IntSymExpr;
  class SymSymExpr;
  some enums, several methods, etc.

As a result we whould accurately define the difference between `Symbol` and `SymExpr`.
I see the next options:

1. `Symbol` and `SymExpr` are different. Leave the names as are. Fix minor mismatches if presented. And follow that definitions.
2. `Symbol` and `SymExpr` are the same. `SymExpr` is a dominant one. Change all the names from `Symbol` to `SymExpr` and get rid of `Symbol`.
3. `Symbol` and `SymExpr` are the same. `Symbol` is a dominant one. Change all the names from `SymExpr` to `Symbol` and get rid of `SymExpr`.
4. Ignore the naming due to loss of **git blame** (or any other reason) like in an llvm naming rules topic.
5. For now just rename `SymbolRef` alias as the beggining of the story in scope of 2nd option.

What do you think?



================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h:111
+using SymExprRef = const SymExpr *;
+using SymbolRefSmallVectorTy = SmallVector<SymExprRef, 2>;
 using SymbolID = unsigned;
----------------
steakhal wrote:
> `SymbolRefSmallVectorTy` -> `SymExprRefSmallVectorTy`
Thanks for a reasonable remark!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89987/new/

https://reviews.llvm.org/D89987



More information about the cfe-commits mailing list