NoQ added inline comments.

================
Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp:153
+// CHECK-NOTES: :[[@LINE-7]]:3: note: Frame #1: function 'boo' calls function 
'bar' here:
+// CHECK-NOTES: :[[@LINE-14]]:18: note: Frame #2: function 'bar' calls 
function 'boo' here:
+// CHECK-NOTES: :[[@LINE-15]]:18: note: ... which was the starting point of 
this call graph
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > NoQ wrote:
> > > Aha, yeah, the warning is present, i guess that part is correct, but look 
> > > how your note `function 'bar' calls function 'boo' here:` doesn't really 
> > > point into the body of 'bar'. In this case it still makes sense because 
> > > it's easy to see that 'foo' is called from 'bar', but the chain of 
> > > default arguments may be arbitrarily long (what if 'boo' has yet another 
> > > default argument?). You might want to add a separate facility just to 
> > > handle this edge case.
> > To make this more readable, the diag is:
> > ```
> > /builddirs/llvm-project/build-Clang9-unknown/tools/clang/tools/extra/test/clang-tidy/checkers/Output/misc-no-recursion.cpp.tmp.cpp:138:5:
> >  warning: function 'boo' is part of call graph loop [misc-no-recursion]
> > int boo();
> >     ^
> > /builddirs/llvm-project/build-Clang9-unknown/tools/clang/tools/extra/test/clang-tidy/checkers/Output/misc-no-recursion.cpp.tmp.cpp:140:6:
> >  warning: function 'bar' is part of call graph loop [misc-no-recursion]
> > void bar() {
> >      ^
> > /builddirs/llvm-project/build-Clang9-unknown/tools/clang/tools/extra/test/clang-tidy/checkers/Output/misc-no-recursion.cpp.tmp.cpp:138:5:
> >  note: Example call graph loop, starting from function 'boo'
> > int boo();
> >     ^
> > /builddirs/llvm-project/build-Clang9-unknown/tools/clang/tools/extra/test/clang-tidy/checkers/Output/misc-no-recursion.cpp.tmp.cpp:145:3:
> >  note: Frame #1: function 'boo' calls function 'bar' here:
> >   bar();
> >   ^
> > /builddirs/llvm-project/build-Clang9-unknown/tools/clang/tools/extra/test/clang-tidy/checkers/Output/misc-no-recursion.cpp.tmp.cpp:139:18:
> >  note: Frame #2: function 'bar' calls function 'boo' here:
> > void foo(int x = boo()) {}
> >                  ^
> > ```
> > Am i looking at this wrong, or all the info is present?
> Edit: right, `bar->foo` edge is not shown there.
Yup. And, well, semantically it's kind of correct, because it's not `foo()` 
that calls `boo()`, but it's `bar()`'s responsiblity to call `boo()` directly 
before calling `foo()`. So it's more like a three-way relationship:
```
misc-no-recursion.cpp.tmp.cpp:139:18: note: Frame #1: function 'bar' calls 
function 'boo' here:
void foo(int x = boo()) {}
                 ^
misc-no-recursion.cpp.tmp.cpp:141:7: note: ...in order to compute default 
argument for function 'foo' here:
  foo();
      ^
```
So i'd imagine `CallRecord`s potentially containing arbitrarily long chains of 
backreferences from a default argument initializer to the respective 
`CXXDefaultArgExpr` that we're currently visiting (which may in turn be part of 
another default argument initializer, given that those are arbitrary 
expressions).

(it's of course up to you whether you want to deal with this immediately or 
later, i'm just ranting, don't mind me)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72362



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to