[PATCH] D19913: Added static creators that create complete instances of SymbolInfo.

[Eric Liu via cfe-commits]

ioeric added inline comments.

================
Comment at: include-fixer/find-all-symbols/SymbolInfo.h:98-101
@@ +97,6 @@
+
+  static SymbolInfo
+  CreateFunctionSymbolInfo(const std::string &Name, const std::string &FilePath,
+                           const std::vector<Context> &Contexts, int LineNumber,
+                           const FunctionInfo &FuncInfo);
+
----------------
klimek wrote:
> hokein wrote:
> > However, changing `SymbolInfo` to class requires us to add many setters/getters in it.
> > 
> > Currently the `SymbolInfo` is like `ClangTidyOptions` structure. 
> So, multiple thoughts:
> - generally, I think symbol info would better be an immutable type; that would mean that we wouldn't have setters, just getters, and initialize everything in the constructor
> - afaik llvm/clang style  doesn't require accessors just because something is a class
> - I believe ClangTidyOptions is a very different use case
> 
But this change would also affect the way SymbolInfo is constructed in FindAllSymbol - class members are retrieved from AST in different locations, and we'd need to store fields in many temp variables, which might be troublesome in this case as well as some other cases. With a `struct` implementation plus static creatrors, we can have some flexibility. But I'm not sure if this makes sense from design perspective. 


http://reviews.llvm.org/D19913