[Lldb-commits] [PATCH] D25592: [LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue class

Ilia K via lldb-commits lldb-commits at lists.llvm.org
Fri Oct 14 01:47:23 PDT 2016


ki.stfu accepted this revision.
ki.stfu added inline comments.
This revision is now accepted and ready to land.


================
Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:21-22
 
+static const char *kUnknownValue = "??";
+static const char *kCompositeValuePlaceholder = "{...}";
+
----------------
enlight wrote:
> ki.stfu wrote:
> > use unnamed namespace here
> I would like to use an anonymous namespace but the [[ http://llvm.org/docs/CodingStandards.html#anonymous-namespaces | LLVM coding standards ]] say
> 
> > make anonymous namespaces as small as possible, and only use them for class declarations
> 
> I don't really buy their reasoning, but we are supposed to be following their style now aren't we?
> 
> kUnresolvedValue/kUnresolvedCompositeValue does sound better, I'll rename them.
Okay, we have to follow the coding style. 


================
Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:22
+static const char *kUnknownValue = "??";
+static const char *kCompositeValuePlaceholder = "{...}";
+
----------------
ki.stfu wrote:
> I dislike Placeholder suffix because the previous one is also a placeholder for unknown values. How about renaming to kUnresolvedValue/kUnresolvedCompositeValue?
Actually I like kUnknownValue and I meant to rename the second one only. But it looks good to me also. 


Repository:
  rL LLVM

https://reviews.llvm.org/D25592





More information about the lldb-commits mailing list