Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I guess that's just clang-format ran over the file? If yes, then LGTM
lldb/include/lldb/Symbol/Type.h | ||
---|---|---|
70–90 | FWIW, we usually update the comments to something less >squished< such as: ///< This type is the type whose UID is m_encoding_uid eEcodingIsUID. /// This type is the type whose UID is m_encoding_uid with the const qualifier added eEncodingIsConstUID, I think this patch is good as-is, but there would be bonus points for fixing up the comment style :) |
lldb/include/lldb/Symbol/Type.h | ||
---|---|---|
70–90 | I meant /// This type is the type whose UID is m_encoding_uid eEcodingIsUID. /// This type is the type whose UID is m_encoding_uid with the const qualifier added eEncodingIsConstUID, |
Can you check now? If looks good, can you land it please?
lldb/include/lldb/Symbol/Type.h | ||
---|---|---|
70–90 | Done |
LGTM modulo the inline comment.
lldb/include/lldb/Symbol/Type.h | ||
---|---|---|
204 | Since you're touching this line... The coding standards say that StringRefs should always be passed by-value:
https://llvm.org/docs/ProgrammersManual.html#the-stringref-class |
I addressed what you requested. Please review. If it is good to land, please do, as I don't have permissions.
lldb/include/lldb/Symbol/Type.h | ||
---|---|---|
207–208 | Looks like this could've broken some tests - might be best to revert and retest? The const StringRef& should've been changed to StringRef per @JDevlieghere's comment, but the non-const StringRef& were/are probably load-bearing - they're the result of calling this function (using mutable ref parameters as "out" parameters) & passing them by value breaks that. |
FWIW, we usually update the comments to something less >squished< such as:
I think this patch is good as-is, but there would be bonus points for fixing up the comment style :)