vrnithinkumar added inline comments.
================
Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:79
+
+// TODO: Enabale this test when "std::swap" is modeled seperately.
+void derefOnStdSwappedNullPtr() {
----------------
NoQ wrote:
> vrnithinkumar wrote:
> > vrnithinkumar wrote:
> > > NoQ wrote:
> > > > Instead of commenting out tests, i recommend testing the incorrect
> > > > behavior (with a FIXME comment telling us why it's incorrect). This way
> > > > we'll be notified when the test is fixed, accidentally or
> > > > intentionally, and also generally that's more testing for everybody.
> > > I have commented out this since right now `std::swap` is using
> > > `unique_ptr.swap`.
> > > So note tag for std::swap is added in header file
> > > `system-header-simulator-cxx.h`
> > > eg:
> > > `system-header-simulator-cxx.h Line 978: Swapped null smart pointer
> > > 'PNull' with smart pointer 'P'`
> > - Is it okay to add a expected-note on `system-header-simulator-cxx.h`?
> Aha, i see.
>
> It's obviously not ok because the header is included by multiple tests but
> only some tests will have the note show up.
>
> The usual solution is to put the expected-note comment in the current test
> file but make it refer to the header. Here's doc from
> https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html:
> > If the diagnostic is generated in a separate file, for example in a shared
> > header file, it may be beneficial to be able to declare the file in which
> > the diagnostic will appear, rather than placing the expected-* directive in
> > the actual file itself. This can be done using the following syntax:
> > ```// expected-error@path/include.h:15 {{error message}}```
>
> It still is a bit of a problem that you have to hardcode the line number (so
> everybody who modifies the header above your note will have to update your
> test) but that's not a big deal.
Thanks!
Added the tags for the line from header file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84600/new/
https://reviews.llvm.org/D84600
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
