NoQ 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() { ---------------- 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. 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