[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