[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd accepted this revision.
compnerd added a subscriber: clayborg.
compnerd added a comment.
This revision is now accepted and ready to land.

Would be nice to get someone like @clayborg to chime in, but, I think that @labath also seems to think that this is fine.



================
Comment at: source/Commands/CommandObjectMemory.cpp:476
+        for (auto lang : Language::GetSupportedLanguages()) {
+          if (PersistentExpressionState *persistent_vars =
+                  target->GetPersistentExpressionStateForLanguage(lang)) {
----------------
I think that `auto` would be fine given that the `GetPersistentExpressionStateForLanguage` spells out the return type.


================
Comment at: source/Commands/CommandObjectMemory.cpp:481
+                        lookup_type_name)) {
+              clang_ast_type = *type;
+              break;
----------------
This seems wrong now.  You iterate over all the languages, but always set the `clang_ast_type`?  The type may be a Swift AST type or Clang AST type (or something more exotic like a rust AST type).  We should rename that to `m_ast_type` as per the LLDB style.  But, that makes sense to do as a follow up change.


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

https://reviews.llvm.org/D62797