aaron.ballman added inline comments.
================ Comment at: test/SemaObjCXX/no-crash-thread-safety-analysis.mm:1 +// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wno-objc-root-class %s + ---------------- aaronpuchert wrote: > jfb wrote: > > aaronpuchert wrote: > > > Test is fine for me, but I would like if you could integrate it with the > > > existing test/SemaObjCXX/warn-thread-safety-analysis.mm. The thread > > > safety analysis requires a bit of setup, that's why we tend to keep the > > > tests in one file. I'll admit that the C++ tests have grown quite large, > > > but for ObjC++ it's still very manageable. > > Sure thing! I created a header that's shared and simplified this repro a > > bit. I validated that the shared code was crashing before and this fixes > > the crash. > I would still like all ObjCXX tests for -Wthread-safety in one file, because > that's what we have for the other languages as well. (more or less) > > It's somewhat easier to see which aspects are tested then and which are not. > Right now we only have tests for reported crashes, but maybe someday someone > finds the time to test a bit more exhaustively. > > But perhaps @aaron.ballman sees this another way, in that case follow his > opinion. On one hand, the all-in-one test files get to be unwieldy size, but on the other hand, it's nice having everything integrated into a single file for testing purposes. I'd say go with the all-in-one approach because it's consistent with how we otherwise test thread safety and there are some (albeit, minor) benefits. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https://reviews.llvm.org/D59523 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
