[Lldb-commits] [PATCH] D13073: Add an expression parser for Go

Bruce Mitchener via lldb-commits lldb-commits at lists.llvm.org
Tue Oct 13 19:55:28 PDT 2015


brucem added a subscriber: brucem.
brucem added a comment.

Hopefully some helpful comments that will help keep this code in line with changes that we're making to other parts of the codebase in bulk ...


================
Comment at: include/lldb/Symbol/GoASTContext.h:394
@@ +393,3 @@
+    }
+    virtual UserExpression *GetUserExpression(const char *expr, const char *expr_prefix, lldb::LanguageType language,
+                                              Expression::ResultType desired_type) override;
----------------
Don't need `virtual` here since it already has `override` (most of the code that has `override` doesn't have `virtual` as well).

================
Comment at: source/Plugins/ExpressionParser/Go/GoAST.h:181
@@ +180,3 @@
+    GoASTArrayType(GoASTExpr* len, GoASTExpr* elt) : GoASTExpr(eArrayType), m_len(len), m_elt(elt) {}
+    virtual ~GoASTArrayType() { }
+
----------------
I think a lot of these destructors could be `~GoASTArrayType override = default;` ? (for this and other classes here)

================
Comment at: source/Plugins/ExpressionParser/Go/GoAST.h:183
@@ +182,3 @@
+
+    const char* GetKindName() const { return "ArrayType"; }
+
----------------
This should have `override` on it.

================
Comment at: source/Plugins/ExpressionParser/Go/GoAST.h:209
@@ +208,3 @@
+
+    const char* GetKindName() const { return "AssignStmt"; }
+
----------------
This should have `override` as well.

================
Comment at: source/Plugins/ExpressionParser/Go/GoAST.h:241
@@ +240,3 @@
+
+    const char* GetKindName() const { return "BadDecl"; }
+
----------------
`override` here too and the rest of the `GetKindName` overrides.

================
Comment at: source/Plugins/ExpressionParser/Go/GoAST.h:2004
@@ +2003,3 @@
+
+    default:
+        break;
----------------
Could this default go away if all of the possible values for that enumeration are covered here? If so, we'd get a compile time warning here when someone adds something to the enumeration.


Repository:
  rL LLVM

http://reviews.llvm.org/D13073





More information about the lldb-commits mailing list