On Wed, Apr 23, 2014 at 11:39 AM, Aaron Ballman <[email protected]>wrote:
> On Wed, Apr 23, 2014 at 2:16 PM, Richard Smith <[email protected]> > wrote: > > On Wed, Apr 23, 2014 at 9:26 AM, Abramo Bagnara < > [email protected]> > > wrote: > >> > >> In r205915 you've added to ThreadSafetyTIL.h a mechanism to avoid > >> explicit delete for SExpr, but this break compilation with REQUIRES_EH=1 > >> as shown below: > >> > >> $ make VERBOSE=1 REQUIRES_EH=1 > >> llvm[0]: Compiling ThreadSafety.cpp for Debug+Asserts build > >> if g++ -Illvm-r206978-build/include > >> -Illvm-r206978-build/tools/clang/lib/Analysis -Illvm-r206978/include > >> -Illvm-r206978/tools/clang/lib/Analysis -D_DEBUG -D_GNU_SOURCE > >> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS > >> -Illvm-r206978/tools/clang/lib/Analysis/../../include > >> -Illvm-r206978-build/tools/clang/lib/Analysis/../../include -g > >> -std=c++11 -fvisibility-inlines-hidden -fPIC -Woverloaded-virtual > >> -ffunction-sections -fdata-sections -Wcast-qual -fno-strict-aliasing > >> -pedantic -Wno-long-long -Wall -W -Wno-unused-parameter -Wwrite-strings > >> -I/home/abramo/eclair_cplusplus/deps/traps/include > >> -Wno-maybe-uninitialized -Wno-missing-field-initializers -c -MMD -MP -MF > >> > >> > "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d.tmp" > >> -MT > >> > "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.o" > >> -MT > >> > "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d" > >> llvm-r206978/tools/clang/lib/Analysis/ThreadSafety.cpp -o > >> > llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.o ; > >> \ > >> then /bin/mv -f > >> > >> > "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d.tmp" > >> > >> > "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d"; > >> else /bin/rm > >> > >> > "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d.tmp"; > >> exit 1; fi > >> In file included from > >> llvm-r206978/tools/clang/lib/Analysis/ThreadSafety.cpp:25:0: > >> > >> > llvm-r206978/tools/clang/lib/Analysis/../../include/clang/Analysis/Analyses/ThreadSafetyTIL.h: > >> In member function ‘clang::threadSafety::til::SExpr* > >> > >> > clang::threadSafety::til::CopyReducer::reduceUndefined(clang::threadSafety::til::Undefined&)’: > >> > >> > llvm-r206978/tools/clang/lib/Analysis/../../include/clang/Analysis/Analyses/ThreadSafetyTIL.h:120:8: > >> error: ‘static void clang::threadSafety::til::SExpr::operator > >> delete(void*)’ is private > >> void operator delete(void *) LLVM_DELETED_FUNCTION; > >> ^ > >> In file included from > >> llvm-r206978/tools/clang/lib/Analysis/ThreadSafety.cpp:26:0: > >> > >> > llvm-r206978/tools/clang/lib/Analysis/../../include/clang/Analysis/Analyses/ThreadSafetyTraverse.h:127:38: > >> error: within this context > >> return new (Arena) Undefined(Orig); > >> ^ > >> ... and many others. > >> > >> I don't think this is expected: what do you think about to remove the > >> private deleted operator delete? Are there better way to fix that? > > > > > > This looks like a GCC bug; it shouldn't be checking accessibility on this > > 'operator delete' here, as far as I can see. The simplest fix is to make > the > > 'operator delete' public -- it's already marked LLVM_DELETED_FUNCTION, > and > > that's sufficient to stop these objects getting accidentally deleted. > > Except on systems like MSVC 2012, where LLVM_DELETED_FUNCTION is a > noop (so it's meant to be a declaration without a definition and > trigger link errors). Though, given the alternative (non-compiling > build for a broken gcc), I think it's a reasonable approach. It's still sufficient to stop these objects getting accidentally deleted if even one person/buildbot builds with a compiler that supports =delete. This is the whole point of LLVM_DELETED_FUNCTION, and we already accept that some errors might make it through to a buildbot in all the other places where we use it.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
