Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land.
Whoo! The patch looks great and well thought out, the tests look like they cover everything and we also talked about plans for future patches. Excellent! I left a nit about merging the test files, but I'll leave it up to you to address or ignore it. ================ Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints.c:1-7 +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \ +// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -triple x86_64-unknown-linux-gnu \ +// RUN: -verify ---------------- martong wrote: > Szelethus wrote: > > Hmm, why do we have 2 different test files that essentially do the same? > > Shouldn't we only have a single one with `analyzer-output=text`? > No, I wanted to have two different test files to test two different things: > (1) We do have the constraints applied (here we don't care about the warnings > and the path) > (2) Check that we have a warning with the proper tracking and notes. What if we had different `-verify`s? `clang/test/Analysis/track-conditions.cpp` is a great example. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73898/new/ https://reviews.llvm.org/D73898 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
