[PATCH] D125782: [llvm-debuginfo-analyzer] 07 - Compare elements

Carlos Alberto Enciso via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 5 04:16:10 PDT 2022


CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVLine.cpp:78
+LVLine *LVLine::findIn(const LVLines *Targets) const {
+  if (Targets) {
+    LLVM_DEBUG({
----------------
probinson wrote:
> ```
> if (!Targets)
>   return nullptr;
> ```
> allows the rest of the function to be less indented.
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:826
+LVScope *LVScope::findIn(const LVScopes *Targets) const {
+  if (Targets) {
+    // In the case of overloaded functions, sometimes the DWARF used to
----------------
probinson wrote:
> Can reduce indentation by starting with
> ```
> if (!Targets)
>   return nullptr;
> ```
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:900
+                                 bool TraverseChildren) {
+  LLVM_DEBUG({
+    if (References && Targets) {
----------------
probinson wrote:
> Could simplify slightly by starting with
> ```
> if (!(References && Targets))
>   return;
> ```
Changed to your suggestion.
Removed the extra check condition.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:917
+  });
+  if (References && Targets)
+    for (LVScope *Reference : *References) {
----------------
probinson wrote:
> If you don't simplify as suggested above, this `if` needs braces around the `for` per LLVM style.
Changed to the previous suggestion.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVSymbol.cpp:285
+                                  const LVSymbols *Targets) {
+  LLVM_DEBUG({
+    if (References && Targets) {
----------------
probinson wrote:
> Could simplify/reduce indentation by starting with
> ```
> if (!(References && Targets))
>   return;
> ```
Changed to your suggestion.
Removed the extra check condition.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVSymbol.cpp:311
+LVSymbol *LVSymbol::findIn(const LVSymbols *Targets) const {
+  if (Targets) {
+    LLVM_DEBUG({
----------------
probinson wrote:
> Could reduce indentation by starting with
> ```
> if (!Targets)
>   return nullptr;
> ```
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp:175
+                                const LVTypes *Targets) {
+  LLVM_DEBUG({
+    if (References && Targets) {
----------------
probinson wrote:
> Could simplify by doing this first:
> ```
> if (!(References && Targets))
>   return;
> ```
Changed to your suggestion.
Removed the extra check condition.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp:201
+LVType *LVType::findIn(const LVTypes *Targets) const {
+  if (Targets) {
+    LLVM_DEBUG({
----------------
probinson wrote:
> Reduce indentation with
> ```
> if (!Targets)
>   return nullptr;
> ```
Changed.


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

https://reviews.llvm.org/D125782



More information about the llvm-commits mailing list