[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:441
+  const SymbolMetadata *getMetadataSymbol(const MemRegion *R, QualType T,
                                           const void *SymbolTag = nullptr);
 
----------------
Why do we even need the tag?
Why is it defaulted to `nullptr` if it asserts later that the `tag` must not be `null`.
I'm confused, somebody help me out :D


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:516-522
   /// Unconditionally marks a symbol as live.
   ///
   /// This should never be
   /// used by checkers, only by the state infrastructure such as the store and
   /// environment. Checkers should instead use metadata symbols and markInUse.
+  /// TODO: update this comment!!!
   void markLive(SymbolRef sym);
----------------
Is it true for modeling checkers too?


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:524-532
   /// Marks a symbol as important to a checker.
   ///
   /// For metadata symbols,
   /// this will keep the symbol alive as long as its associated region is also
   /// live. For other symbols, this has no effect; checkers are not permitted
   /// to influence the life of other symbols. This should be used before any
   /// symbol marking has occurred, i.e. in the MarkLiveSymbols callback.
----------------
I think it should be removed, but for now I left it to draw attention in the review process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86445