[PATCH] D55953: Android is not GNU, so don't claim that it is.
kristina added a comment. Seems good, it could eliminate the need for a lot of preprocessor checks like `#if __gnu_linux__ && !defined(__ANDROID__)`. It doesn't seem that there are any preprocessor checks where this would cause problems (from a quick search) since all of them seem to focus on making sure `__gnu_linux__` being defined explicitly excludes Android from those paths. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55953/new/ https://reviews.llvm.org/D55953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56158: [sanitizer_common] Implement funopen*() interceptors for NetBSD
mgorny updated this revision to Diff 179722. mgorny added a comment. Implemented wrappers as requested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56158/new/ https://reviews.llvm.org/D56158 Files: lib/sanitizer_common/sanitizer_common_interceptors.inc lib/sanitizer_common/sanitizer_platform_interceptors.h lib/sanitizer_common/sanitizer_platform_limits_netbsd.h Index: lib/sanitizer_common/sanitizer_platform_limits_netbsd.h === --- lib/sanitizer_common/sanitizer_platform_limits_netbsd.h +++ lib/sanitizer_common/sanitizer_platform_limits_netbsd.h @@ -2319,6 +2319,7 @@ void *hash; uptr key_counter; }; + } // namespace __sanitizer #define CHECK_TYPE_SIZE(TYPE) \ Index: lib/sanitizer_common/sanitizer_platform_interceptors.h === --- lib/sanitizer_common/sanitizer_platform_interceptors.h +++ lib/sanitizer_common/sanitizer_platform_interceptors.h @@ -549,5 +549,7 @@ #define SANITIZER_INTERCEPT_POPEN SI_POSIX #define SANITIZER_INTERCEPT_POPENVE SI_NETBSD #define SANITIZER_INTERCEPT_PCLOSE SI_POSIX +#define SANITIZER_INTERCEPT_FUNOPEN SI_NETBSD +#define SANITIZER_INTERCEPT_FUNOPEN2 SI_NETBSD #endif // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H Index: lib/sanitizer_common/sanitizer_common_interceptors.inc === --- lib/sanitizer_common/sanitizer_common_interceptors.inc +++ lib/sanitizer_common/sanitizer_common_interceptors.inc @@ -9133,6 +9133,161 @@ #define INIT_PCLOSE #endif +#if SANITIZER_INTERCEPT_FUNOPEN +typedef int (*funopen_readfn)(void *cookie, char *buf, int len); +typedef int (*funopen_writefn)(void *cookie, const char *buf, int len); +typedef OFF_T (*funopen_seekfn)(void *cookie, OFF_T offset, int whence); +typedef int (*funopen_closefn)(void *cookie); + +struct WrappedCookie { + void *real_cookie; + funopen_readfn real_read; + funopen_writefn real_write; + funopen_seekfn real_seek; + funopen_closefn real_close; +}; + +static int wrapped_read(void *cookie, char *buf, int len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedCookie *wrapped_cookie = (WrappedCookie *)cookie; + funopen_readfn real_read = wrapped_cookie->real_read; + return real_read(wrapped_cookie->real_cookie, buf, len); +} + +static int wrapped_write(void *cookie, const char *buf, int len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedCookie *wrapped_cookie = (WrappedCookie *)cookie; + funopen_writefn real_write = wrapped_cookie->real_write; + return real_write(wrapped_cookie->real_cookie, buf, len); +} + +static OFF_T wrapped_seek(void *cookie, OFF_T offset, int whence) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedCookie *wrapped_cookie = (WrappedCookie *)cookie; + funopen_seekfn real_seek = wrapped_cookie->real_seek; + return real_seek(wrapped_cookie->real_cookie, offset, whence); +} + +static int wrapped_close(void *cookie) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(1); + WrappedCookie *wrapped_cookie = (WrappedCookie *)cookie; + funopen_closefn real_close = wrapped_cookie->real_close; + int res = real_close(wrapped_cookie->real_cookie); + InternalFree(wrapped_cookie); + return res; +} + +INTERCEPTOR(__sanitizer_FILE *, funopen, void *cookie, funopen_readfn readfn, +funopen_writefn writefn, funopen_seekfn seekfn, +funopen_closefn closefn) { + void *ctx; + COMMON_INTERCEPTOR_ENTER(ctx, funopen, cookie, readfn, writefn, + seekfn, closefn); + + WrappedCookie *wrapped_cookie = + (WrappedCookie *)InternalAlloc(sizeof(WrappedCookie)); + wrapped_cookie->real_cookie = cookie; + wrapped_cookie->real_read = readfn; + wrapped_cookie->real_write = writefn; + wrapped_cookie->real_seek = seekfn; + wrapped_cookie->real_close = closefn; + + __sanitizer_FILE *res = REAL(funopen)(wrapped_cookie, +readfn ? wrapped_read : nullptr, +writefn ? wrapped_write : nullptr, +seekfn ? wrapped_seek : nullptr, +closefn ? wrapped_close : nullptr); + if (res) unpoison_file(res); + return res; +} +#define INIT_FUNOPEN COMMON_INTERCEPT_FUNCTION(funopen) +#else +#define INIT_FUNOPEN +#endif + +#if SANITIZER_INTERCEPT_FUNOPEN2 +typedef SSIZE_T (*funopen2_readfn)(void *cookie, void *buf, SIZE_T len); +typedef SSIZE_T (*funopen2_writefn)(void *cookie, const void *buf, SIZE_T len); +typedef OFF_T (*funopen2_seekfn)(void *cookie, OFF_T offset, int whence); +typedef int (*funopen2_flushfn)(void *cookie); +typedef int (*funopen2_closefn)(void *cookie); + +struct WrappedCookie2 { + void *real_cookie; + funopen2_readfn real_read; + funopen2_writefn real_write; + funopen2_seekfn real_seek; + funopen2_flushfn real_flush; + funopen2_closefn real_close; +}; + +static SSIZE_T wrapped_read2(void
[PATCH] D56158: [sanitizer_common] Implement funopen*() interceptors for NetBSD
mgorny updated this revision to Diff 179723. mgorny added a comment. Fixed accidental whitespace change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56158/new/ https://reviews.llvm.org/D56158 Files: lib/sanitizer_common/sanitizer_common_interceptors.inc lib/sanitizer_common/sanitizer_platform_interceptors.h Index: lib/sanitizer_common/sanitizer_platform_interceptors.h === --- lib/sanitizer_common/sanitizer_platform_interceptors.h +++ lib/sanitizer_common/sanitizer_platform_interceptors.h @@ -549,5 +549,7 @@ #define SANITIZER_INTERCEPT_POPEN SI_POSIX #define SANITIZER_INTERCEPT_POPENVE SI_NETBSD #define SANITIZER_INTERCEPT_PCLOSE SI_POSIX +#define SANITIZER_INTERCEPT_FUNOPEN SI_NETBSD +#define SANITIZER_INTERCEPT_FUNOPEN2 SI_NETBSD #endif // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H Index: lib/sanitizer_common/sanitizer_common_interceptors.inc === --- lib/sanitizer_common/sanitizer_common_interceptors.inc +++ lib/sanitizer_common/sanitizer_common_interceptors.inc @@ -9133,6 +9133,161 @@ #define INIT_PCLOSE #endif +#if SANITIZER_INTERCEPT_FUNOPEN +typedef int (*funopen_readfn)(void *cookie, char *buf, int len); +typedef int (*funopen_writefn)(void *cookie, const char *buf, int len); +typedef OFF_T (*funopen_seekfn)(void *cookie, OFF_T offset, int whence); +typedef int (*funopen_closefn)(void *cookie); + +struct WrappedCookie { + void *real_cookie; + funopen_readfn real_read; + funopen_writefn real_write; + funopen_seekfn real_seek; + funopen_closefn real_close; +}; + +static int wrapped_read(void *cookie, char *buf, int len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedCookie *wrapped_cookie = (WrappedCookie *)cookie; + funopen_readfn real_read = wrapped_cookie->real_read; + return real_read(wrapped_cookie->real_cookie, buf, len); +} + +static int wrapped_write(void *cookie, const char *buf, int len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedCookie *wrapped_cookie = (WrappedCookie *)cookie; + funopen_writefn real_write = wrapped_cookie->real_write; + return real_write(wrapped_cookie->real_cookie, buf, len); +} + +static OFF_T wrapped_seek(void *cookie, OFF_T offset, int whence) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedCookie *wrapped_cookie = (WrappedCookie *)cookie; + funopen_seekfn real_seek = wrapped_cookie->real_seek; + return real_seek(wrapped_cookie->real_cookie, offset, whence); +} + +static int wrapped_close(void *cookie) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(1); + WrappedCookie *wrapped_cookie = (WrappedCookie *)cookie; + funopen_closefn real_close = wrapped_cookie->real_close; + int res = real_close(wrapped_cookie->real_cookie); + InternalFree(wrapped_cookie); + return res; +} + +INTERCEPTOR(__sanitizer_FILE *, funopen, void *cookie, funopen_readfn readfn, +funopen_writefn writefn, funopen_seekfn seekfn, +funopen_closefn closefn) { + void *ctx; + COMMON_INTERCEPTOR_ENTER(ctx, funopen, cookie, readfn, writefn, + seekfn, closefn); + + WrappedCookie *wrapped_cookie = + (WrappedCookie *)InternalAlloc(sizeof(WrappedCookie)); + wrapped_cookie->real_cookie = cookie; + wrapped_cookie->real_read = readfn; + wrapped_cookie->real_write = writefn; + wrapped_cookie->real_seek = seekfn; + wrapped_cookie->real_close = closefn; + + __sanitizer_FILE *res = REAL(funopen)(wrapped_cookie, +readfn ? wrapped_read : nullptr, +writefn ? wrapped_write : nullptr, +seekfn ? wrapped_seek : nullptr, +closefn ? wrapped_close : nullptr); + if (res) unpoison_file(res); + return res; +} +#define INIT_FUNOPEN COMMON_INTERCEPT_FUNCTION(funopen) +#else +#define INIT_FUNOPEN +#endif + +#if SANITIZER_INTERCEPT_FUNOPEN2 +typedef SSIZE_T (*funopen2_readfn)(void *cookie, void *buf, SIZE_T len); +typedef SSIZE_T (*funopen2_writefn)(void *cookie, const void *buf, SIZE_T len); +typedef OFF_T (*funopen2_seekfn)(void *cookie, OFF_T offset, int whence); +typedef int (*funopen2_flushfn)(void *cookie); +typedef int (*funopen2_closefn)(void *cookie); + +struct WrappedCookie2 { + void *real_cookie; + funopen2_readfn real_read; + funopen2_writefn real_write; + funopen2_seekfn real_seek; + funopen2_flushfn real_flush; + funopen2_closefn real_close; +}; + +static SSIZE_T wrapped_read2(void *cookie, void *buf, SIZE_T len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedCookie2 *wrapped_cookie = (WrappedCookie2 *)cookie; + funopen2_readfn real_read = wrapped_cookie->real_read; + return real_read(wrapped_cookie->real_cookie, buf, len); +} + +static SSIZE_T wrapped_write2(void *cookie, const void *buf, SIZE_T len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedCookie2 *wrapped_cookie = (WrappedCookie2 *)cook
[PATCH] D54408: Add matchers available through casting to derived
steveire updated this revision to Diff 179725. steveire added a comment. Some updates Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54408/new/ https://reviews.llvm.org/D54408 Files: lib/ASTMatchers/Dynamic/Registry.cpp unittests/ASTMatchers/Dynamic/RegistryTest.cpp Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp === --- unittests/ASTMatchers/Dynamic/RegistryTest.cpp +++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp @@ -461,6 +461,16 @@ "Matcher " "hasDescendant(Matcher)")); + CompVector FuncDeclComps = getCompletions("functionDecl", 0); + + EXPECT_TRUE(!FuncDeclComps.empty()); + + // Exclude bases + EXPECT_TRUE(!hasCompletion(FuncDeclComps, "namedDecl(")); + EXPECT_TRUE(!hasCompletion(FuncDeclComps, "valueDecl(")); + EXPECT_TRUE(!hasCompletion(FuncDeclComps, "declaratorDecl(")); + EXPECT_TRUE(!hasCompletion(FuncDeclComps, "functionDecl(")); + CompVector WhileComps = getCompletions("whileStmt", 0); EXPECT_TRUE(hasCompletion(WhileComps, "hasBody(", @@ -569,13 +579,13 @@ EXPECT_TRUE(Contains(Matchers, "isPublic()")); EXPECT_TRUE(Contains(Matchers, "hasName()")); EXPECT_TRUE(Contains(Matchers, "parameterCountIs()")); + EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl(isOverride())")); EXPECT_FALSE(Contains(Matchers, "namedDecl()")); EXPECT_FALSE(Contains(Matchers, "valueDecl()")); EXPECT_FALSE(Contains(Matchers, "declaratorDecl()")); EXPECT_FALSE(Contains(Matchers, "functionDecl()")); - - EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()")); + EXPECT_FALSE(Contains(Matchers, "cxxMethodDecl()")); } } // end anonymous namespace Index: lib/ASTMatchers/Dynamic/Registry.cpp === --- lib/ASTMatchers/Dynamic/Registry.cpp +++ lib/ASTMatchers/Dynamic/Registry.cpp @@ -647,17 +647,79 @@ } } +llvm::Optional> +getNodeConstructorType(MatcherCtor targetCtor) { + const auto &ctors = RegistryData->nodeConstructors(); + auto it = llvm::find_if( + ctors, [targetCtor](const NodeConstructorMap::value_type &ctor) { +return ctor.second.second == targetCtor; + }); + if (it == ctors.end()) { +return llvm::None; + } + return std::make_pair(it->first, it->second.first); +} + std::vector -Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) { +getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name); + +enum MatchScope { ExactOnly, IncludeDerived }; + +std::vector +getMatchingMatchersImpl(ast_type_traits::ASTNodeKind StaticType, +MatchScope Scope) { + std::vector Result; std::vector AcceptedTypes{StaticType}; processAcceptableMatchers( - AcceptedTypes, - [&Result](StringRef Name, const MatcherDescriptor &, -std::set &, std::vector>, -unsigned) { Result.emplace_back((Name + "()").str()); }); + AcceptedTypes, [&](StringRef Name, const MatcherDescriptor &Matcher, + std::set &, + std::vector>, unsigned) { +unsigned Specificity; +ASTNodeKind LeastDerivedKind; +if (Scope == ExactOnly) { + if (Matcher.isConvertibleTo(StaticType, &Specificity, + &LeastDerivedKind) && + !LeastDerivedKind.isSame(StaticType)) +return; +} + +auto TypeForMatcherOpt = getNodeConstructorType(&Matcher); +if (TypeForMatcherOpt && +StaticType.isBaseOf(TypeForMatcherOpt->first)) { + auto Derived = getDerivedResults(TypeForMatcherOpt->first, Name); + Result.insert(Result.end(), Derived.begin(), Derived.end()); + return; +} + +Result.emplace_back((Name + "()").str()); + }); + + return Result; +} + +std::vector +getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name) { + auto NestedResult = getMatchingMatchersImpl(StaticType, ExactOnly); + + std::vector Result; + + Diagnostics DiagnosticIgnorer; + + for (const auto &item : NestedResult) { +Result.emplace_back((Name + "(" + item.MatcherString + ")").str()); + } + + return Result; +} + +std::vector +Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) { + + std::vector Result = + getMatchingMatchersImpl(StaticType, IncludeDerived); return Result; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived
lebedev.ri added a comment. Differential lacks description. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54408/new/ https://reviews.llvm.org/D54408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56158: [sanitizer_common] Implement funopen*() interceptors for NetBSD
krytarowski added inline comments. Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:9142 + +struct WrappedCookie { + void *real_cookie; I would call it WrappedFunopenCookie to be less generic, similarly for funopen2. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56158/new/ https://reviews.llvm.org/D56158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56158: [sanitizer_common] Implement funopen*() interceptors for NetBSD
krytarowski added inline comments. Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:9150 + +static int wrapped_read(void *cookie, char *buf, int len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); How about `wrapped_funopen_read` etc? It won't clash with future potential inteceptors with so generic names. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56158/new/ https://reviews.llvm.org/D56158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56158: [sanitizer_common] Implement funopen*() interceptors for NetBSD
mgorny updated this revision to Diff 179728. mgorny added a comment. Renamed stuff as requested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56158/new/ https://reviews.llvm.org/D56158 Files: lib/sanitizer_common/sanitizer_common_interceptors.inc lib/sanitizer_common/sanitizer_platform_interceptors.h Index: lib/sanitizer_common/sanitizer_platform_interceptors.h === --- lib/sanitizer_common/sanitizer_platform_interceptors.h +++ lib/sanitizer_common/sanitizer_platform_interceptors.h @@ -549,5 +549,7 @@ #define SANITIZER_INTERCEPT_POPEN SI_POSIX #define SANITIZER_INTERCEPT_POPENVE SI_NETBSD #define SANITIZER_INTERCEPT_PCLOSE SI_POSIX +#define SANITIZER_INTERCEPT_FUNOPEN SI_NETBSD +#define SANITIZER_INTERCEPT_FUNOPEN2 SI_NETBSD #endif // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H Index: lib/sanitizer_common/sanitizer_common_interceptors.inc === --- lib/sanitizer_common/sanitizer_common_interceptors.inc +++ lib/sanitizer_common/sanitizer_common_interceptors.inc @@ -9133,6 +9133,166 @@ #define INIT_PCLOSE #endif +#if SANITIZER_INTERCEPT_FUNOPEN +typedef int (*funopen_readfn)(void *cookie, char *buf, int len); +typedef int (*funopen_writefn)(void *cookie, const char *buf, int len); +typedef OFF_T (*funopen_seekfn)(void *cookie, OFF_T offset, int whence); +typedef int (*funopen_closefn)(void *cookie); + +struct WrappedFunopenCookie { + void *real_cookie; + funopen_readfn real_read; + funopen_writefn real_write; + funopen_seekfn real_seek; + funopen_closefn real_close; +}; + +static int wrapped_funopen_read(void *cookie, char *buf, int len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedFunopenCookie *wrapped_cookie = (WrappedFunopenCookie *)cookie; + funopen_readfn real_read = wrapped_cookie->real_read; + return real_read(wrapped_cookie->real_cookie, buf, len); +} + +static int wrapped_funopen_write(void *cookie, const char *buf, int len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedFunopenCookie *wrapped_cookie = (WrappedFunopenCookie *)cookie; + funopen_writefn real_write = wrapped_cookie->real_write; + return real_write(wrapped_cookie->real_cookie, buf, len); +} + +static OFF_T wrapped_funopen_seek(void *cookie, OFF_T offset, int whence) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedFunopenCookie *wrapped_cookie = (WrappedFunopenCookie *)cookie; + funopen_seekfn real_seek = wrapped_cookie->real_seek; + return real_seek(wrapped_cookie->real_cookie, offset, whence); +} + +static int wrapped_funopen_close(void *cookie) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(1); + WrappedFunopenCookie *wrapped_cookie = (WrappedFunopenCookie *)cookie; + funopen_closefn real_close = wrapped_cookie->real_close; + int res = real_close(wrapped_cookie->real_cookie); + InternalFree(wrapped_cookie); + return res; +} + +INTERCEPTOR(__sanitizer_FILE *, funopen, void *cookie, funopen_readfn readfn, +funopen_writefn writefn, funopen_seekfn seekfn, +funopen_closefn closefn) { + void *ctx; + COMMON_INTERCEPTOR_ENTER(ctx, funopen, cookie, readfn, writefn, seekfn, + closefn); + + WrappedFunopenCookie *wrapped_cookie = + (WrappedFunopenCookie *)InternalAlloc(sizeof(WrappedFunopenCookie)); + wrapped_cookie->real_cookie = cookie; + wrapped_cookie->real_read = readfn; + wrapped_cookie->real_write = writefn; + wrapped_cookie->real_seek = seekfn; + wrapped_cookie->real_close = closefn; + + __sanitizer_FILE *res = + REAL(funopen)(wrapped_cookie, +readfn ? wrapped_funopen_read : nullptr, +writefn ? wrapped_funopen_write : nullptr, +seekfn ? wrapped_funopen_seek : nullptr, +closefn ? wrapped_funopen_close : nullptr); + if (res) +unpoison_file(res); + return res; +} +#define INIT_FUNOPEN COMMON_INTERCEPT_FUNCTION(funopen) +#else +#define INIT_FUNOPEN +#endif + +#if SANITIZER_INTERCEPT_FUNOPEN2 +typedef SSIZE_T (*funopen2_readfn)(void *cookie, void *buf, SIZE_T len); +typedef SSIZE_T (*funopen2_writefn)(void *cookie, const void *buf, SIZE_T len); +typedef OFF_T (*funopen2_seekfn)(void *cookie, OFF_T offset, int whence); +typedef int (*funopen2_flushfn)(void *cookie); +typedef int (*funopen2_closefn)(void *cookie); + +struct WrappedFunopen2Cookie { + void *real_cookie; + funopen2_readfn real_read; + funopen2_writefn real_write; + funopen2_seekfn real_seek; + funopen2_flushfn real_flush; + funopen2_closefn real_close; +}; + +static SSIZE_T wrapped_funopen2_read(void *cookie, void *buf, SIZE_T len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); + WrappedFunopen2Cookie *wrapped_cookie = (WrappedFunopen2Cookie *)cookie; + funopen2_readfn real_read = wrapped_cookie->real_read; + return real_read(wrapped_cookie->real_cookie, buf, len); +} + +static SSIZE_T wrapped_funopen2_write(void *cookie, const void *buf, +
[PATCH] D56160: created clang-tidy pass modernize-use-trailing-return
bernhardmgruber created this revision. bernhardmgruber added a reviewer: alexfh. bernhardmgruber added a project: clang-tools-extra. Herald added subscribers: cfe-commits, mgorny. The new clang-tidy pass modernize-use-trailing-return rewrites function signatures to use a trailing return type. A fair amount of tests are included. Does not work on return types which span locations before and after the function name (e.g. functions returning function pointers). The pass may fail if the return types are from missing headers (e.g. when clang-tidy is run without a compilation database or all needed include directories) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D56160 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/UseTrailingReturnCheck.cpp clang-tidy/modernize/UseTrailingReturnCheck.h docs/clang-tidy/checks/modernize-use-trailing-return.rst test/clang-tidy/modernize-use-trailing-return.cpp Index: test/clang-tidy/modernize-use-trailing-return.cpp === --- /dev/null +++ test/clang-tidy/modernize-use-trailing-return.cpp @@ -0,0 +1,144 @@ +// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14 + +namespace std { +template +class vector; + +class string; +} + +// +// Samples triggering the check +// + +int f(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f() -> int;{{$}} +int f(int); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f(int) -> int;{{$}} +int f(int arg); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f(int arg) -> int;{{$}} +int f(int arg1, int arg2, int arg3); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3) -> int;{{$}} +int f(int arg1, int arg2, int arg3, ...); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3, ...) -> int;{{$}} +template int f(T t); +// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}template auto f(T t) -> int;{{$}} +int a1() { return 42; } +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto a1() -> int { return 42; }{{$}} +int a2() { +return 42; +} +// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto a2() -> int {{{$}} +int a3() +{ +return 42; +} +// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto a3() -> int{{$}} +int b(int arg ) ; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto b(int arg ) -> int ;{{$}} +inline int d1(int arg); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto d1(int arg) -> int;{{$}} +inline int d2(int arg) noexcept(true); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto d2(int arg) -> int noexcept(true);{{$}} +inline int d3(int arg) try { } catch(...) { } +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto d3(int arg) -> int try { } catch(...) { }{{$}} +namespace N { +bool e1(); +} +// CHECK-MESSAGES: :[[@LINE-2]]:10: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto e1() -> bool;{{$}} +inline volatile const std::vector e2(); +// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto e2() -> volatile const std::vector;{{$}} +inline const std::vector volatile e2(); +// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto e2() -> const std::vector volatile;{{$}} +inline std::vector const volatile e2(); +// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return] +//
[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check
lebedev.ri added a comment. Please always upload all patches with full context (`-U9`). Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check
bernhardmgruber added a comment. Hi! This is my first contribution to LLVM and I may not yet know the conventions here. I decided to write this pass, as some work colleagues and me oftenly want to modernize legacy code bases. We like the trailing return type syntax available since C++11 and use it for new code. To keep a consistent code base, it would help a lot to automatically convert pre C++11 code to the new trailing return syntax. I strongly believe this will be handy for several other developers out there, although it probably should not be enabled by default. I am happy to receive any feedback! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check
bernhardmgruber updated this revision to Diff 179734. bernhardmgruber added a comment. updated diff to one with full context CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/UseTrailingReturnCheck.cpp clang-tidy/modernize/UseTrailingReturnCheck.h docs/clang-tidy/checks/modernize-use-trailing-return.rst test/clang-tidy/modernize-use-trailing-return.cpp Index: test/clang-tidy/modernize-use-trailing-return.cpp === --- /dev/null +++ test/clang-tidy/modernize-use-trailing-return.cpp @@ -0,0 +1,144 @@ +// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14 + +namespace std { +template +class vector; + +class string; +} + +// +// Samples triggering the check +// + +int f(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f() -> int;{{$}} +int f(int); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f(int) -> int;{{$}} +int f(int arg); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f(int arg) -> int;{{$}} +int f(int arg1, int arg2, int arg3); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3) -> int;{{$}} +int f(int arg1, int arg2, int arg3, ...); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3, ...) -> int;{{$}} +template int f(T t); +// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}template auto f(T t) -> int;{{$}} +int a1() { return 42; } +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto a1() -> int { return 42; }{{$}} +int a2() { +return 42; +} +// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto a2() -> int {{{$}} +int a3() +{ +return 42; +} +// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto a3() -> int{{$}} +int b(int arg ) ; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto b(int arg ) -> int ;{{$}} +inline int d1(int arg); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto d1(int arg) -> int;{{$}} +inline int d2(int arg) noexcept(true); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto d2(int arg) -> int noexcept(true);{{$}} +inline int d3(int arg) try { } catch(...) { } +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto d3(int arg) -> int try { } catch(...) { }{{$}} +namespace N { +bool e1(); +} +// CHECK-MESSAGES: :[[@LINE-2]]:10: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto e1() -> bool;{{$}} +inline volatile const std::vector e2(); +// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto e2() -> volatile const std::vector;{{$}} +inline const std::vector volatile e2(); +// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto e2() -> const std::vector volatile;{{$}} +inline std::vector const volatile e2(); +// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}inline auto e2() -> std::vector const volatile;{{$}} +bool N::e1() {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use a trailing return type for this function [modernize-use-trailing-return] +// CHECK-FIXES: {{^}}auto N::e1() -> bool {}{{$}} +int (*e3())(double); +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a trailing return type for this function (FixIt not implemented) [modernize-use-trailing-return] +// TODO: not matched by the AST matcher +//d
[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check
lebedev.ri added a comment. Some thoughts. Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:21-22 + +// very similar to UseOverrideCheck +SourceLocation findTrailingReturnTypeLocation(const CharSourceRange &range, + const ASTContext &ctx, This function looks quite fragile. Is there no existing function elsewhere [in the support libraries] that does this? There is no way to extract this from AST? Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:72 + + llvm_unreachable("Expected to find a closing paranthesis"); +} parenthesis Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:75 + +SourceRange findReturnTypeAndCVRange(const FunctionDecl &f, + const ASTContext &ctx, Same question, really. Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:82 + // and volatile + auto returnTypeRange = f.getReturnTypeSourceRange(); + Is this `SourceRange`? Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:91 + Token t; + std::vector tokens; + while (!lexer.LexFromRawLexer(t)) { Wouldn't `SmallVector` be sufficient in most cases? Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:161-163 + const auto &ctx = *Result.Context; + const auto &sm = *Result.SourceManager; + const auto &opts = getLangOpts(); 1. All the variables should be WithThisCase 2. Can't tell if the `auto` is ok here? Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:165 + + auto returnTypeCVRange = findReturnTypeAndCVRange(*f, ctx, sm, opts); + if (!returnTypeCVRange.isValid()) { It might be better to drop `Range` suffix, and spell the expected return type `SourceRange` explicitly. Or add `Source` word :) Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:178-179 +return; + const auto insertionLoc = + findTrailingReturnTypeLocation(charRange, ctx, sm, opts); + Same, i'd say either add `Source`, or drop `Location` and spell it out completely. Comment at: docs/clang-tidy/checks/modernize-use-trailing-return.rst:4 +modernize-use-trailing-return +== + Please pad with extra `=` (to align the length) Comment at: docs/clang-tidy/checks/modernize-use-trailing-return.rst:7 + +Rewrites function signatures to use a trailing return type. Needs better docs, ideally with some examples Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:2 +// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14 + +namespace std { Missing test coverage: * macros * is there tests for implicit functions? * Out-of-line function with body. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56116: [gn build] Make `ninja check-clang` also run Clang's unit tests
thakis updated this revision to Diff 179736. thakis edited the summary of this revision. thakis added a comment. Add unittest template. This is now ready to go. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56116/new/ https://reviews.llvm.org/D56116 Files: llvm/utils/gn/build/BUILDCONFIG.gn llvm/utils/gn/build/sync_source_lists_from_cmake.py llvm/utils/gn/secondary/clang/lib/ASTMatchers/Dynamic/BUILD.gn llvm/utils/gn/secondary/clang/test/BUILD.gn llvm/utils/gn/secondary/clang/unittests/AST/BUILD.gn llvm/utils/gn/secondary/clang/unittests/ASTMatchers/BUILD.gn llvm/utils/gn/secondary/clang/unittests/ASTMatchers/Dynamic/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Analysis/BUILD.gn llvm/utils/gn/secondary/clang/unittests/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Basic/BUILD.gn llvm/utils/gn/secondary/clang/unittests/CodeGen/BUILD.gn llvm/utils/gn/secondary/clang/unittests/CrossTU/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Driver/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Frontend/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Index/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Lex/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Rename/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Rewrite/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Sema/BUILD.gn llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Tooling/BUILD.gn llvm/utils/gn/secondary/clang/unittests/libclang/BUILD.gn llvm/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn llvm/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni Index: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni === --- /dev/null +++ llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni @@ -0,0 +1,44 @@ +# This file defines a template for adding a unittest binary. +# +# It's a thin wrapper around GN's built-in executable() target type and +# accepts the same parameters. +# +# Example use: +# +# unittest("FormatTest") { +# sources = [ ... ] +# ... +# } + +template("unittest") { + executable(target_name) { +# Foward everything (configs, sources, deps, ...). +forward_variables_from(invoker, "*") + +# Common settings for all unit tests. +# Unit test binaries shouldn't go right in out/gn/bin, for two reasons: +# 1. That's where production binaries go. +# 2. The CMake build doesn't put the unit tests of all projects (clang, +#lld,...) in one directory, so it's not guaranteed that there won't +#be name collisions between test binaries from separate projects. +# Each lit suite takes an foo_obj_root parameter and puts temporary files +# for lit tests at foo_obj_root/test and looks for unit test binaries +# below foo_obj_root/unittests. As long as the BUILD.gn files processing +# the lit.site.cfg.py.in files match the output dir here, it doesn't +# matter all that much where the unit test binaries go, with the weak +# constraints that test binaries of different projects should go in +# different folders, and that it's not too difficult to manually +# run the unit test binary if necessary. Using target_out_dir here +# means that //clang/unittests/Format gets its binary in +# out/gn/obj/clang/unittests/Format/FormatTests, which seems fine. +output_dir = target_out_dir +deps += [ + "//llvm/utils/unittest/UnitTestMain", +] +testonly = true + } +} + +set_defaults("unittest") { + configs = shared_binary_target_configs +} Index: llvm/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn === --- llvm/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn +++ llvm/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn @@ -1,13 +1,11 @@ -executable("MachOTests") { - # test/Unit/lit.cfg expects unittests in LLD_BINARY_DIR/unittest - output_dir = target_out_dir +import("//llvm/utils/unittest/unittest.gni") +unittest("MachOTests") { configs += [ "//llvm/utils/gn/build:lld_code" ] deps = [ "//lld/lib/Driver", "//lld/lib/ReaderWriter/MachO", "//lld/lib/ReaderWriter/YAML", -"//llvm/utils/unittest/UnitTestMain", ] sources = [ "MachONormalizedFileBinaryReaderTests.cpp", @@ -15,5 +13,4 @@ "MachONormalizedFileToAtomsTests.cpp", "MachONormalizedFileYAMLTests.cpp", ] - testonly = true } Index: llvm/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn === --- llvm/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn +++ llvm/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn @@ -1,15 +1,12 @@ -executable("DriverTests") { - # test/Unit/lit.cfg expects unittests in LLD_BINARY_DIR/unitte
[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check
Eugene.Zelenko added a comment. Please mention new check in Release Notes and list of checks. It'll be good idea to used add_new_check.py. Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:22 +// very similar to UseOverrideCheck +SourceLocation findTrailingReturnTypeLocation(const CharSourceRange &range, + const ASTContext &ctx, Please use static instead of anonymous namespace. Same for other function. See LLVM coding guidelines. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r350167 - DeclAccessPair visualizer should be expandable
Author: mps Date: Sun Dec 30 12:22:37 2018 New Revision: 350167 URL: http://llvm.org/viewvc/llvm-project?rev=350167&view=rev Log: DeclAccessPair visualizer should be expandable Modified: cfe/trunk/utils/ClangVisualizers/clang.natvis Modified: cfe/trunk/utils/ClangVisualizers/clang.natvis URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/ClangVisualizers/clang.natvis?rev=350167&r1=350166&r2=350167&view=diff == --- cfe/trunk/utils/ClangVisualizers/clang.natvis (original) +++ cfe/trunk/utils/ClangVisualizers/clang.natvis Sun Dec 30 12:22:37 2018 @@ -553,6 +553,10 @@ For later versions of Visual Studio, no b {*(clang::NamedDecl *)(Ptr&~Mask)} {*this,view(access)} {*this,view(decl)} + + (clang::AccessSpecifier)(Ptr&Mask),en + *(clang::NamedDecl *)(Ptr&~Mask) + {Decls} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56116: [gn build] Make `ninja check-clang` also run Clang's unit tests
phosek added inline comments. Comment at: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni:34 +# out/gn/obj/clang/unittests/Format/FormatTests, which seems fine. +output_dir = target_out_dir +deps += [ What if someone explicitly sets `output_dir` in the invoker? We should either preserve that value or assert with an error message that overriding `output_dir` in `unittest`s is unsupported. Comment at: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni:35 +output_dir = target_out_dir +deps += [ + "//llvm/utils/unittest/UnitTestMain", You should check if `deps` is defined and set `deps = []` if not, otherwise the attempt to add to non-existent list is going to fail. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56116/new/ https://reviews.llvm.org/D56116 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56116: [gn build] Make `ninja check-clang` also run Clang's unit tests
thakis added a comment. Thanks for the fast turnaround! Comment at: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni:34 +# out/gn/obj/clang/unittests/Format/FormatTests, which seems fine. +output_dir = target_out_dir +deps += [ phosek wrote: > What if someone explicitly sets `output_dir` in the invoker? We should either > preserve that value or assert with an error message that overriding > `output_dir` in `unittest`s is unsupported. Good point, done. Comment at: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni:35 +output_dir = target_out_dir +deps += [ + "//llvm/utils/unittest/UnitTestMain", phosek wrote: > You should check if `deps` is defined and set `deps = []` if not, otherwise > the attempt to add to non-existent list is going to fail. I can't imagine that anyone would want a unit test that doesn't at least depend on the code under test. I agree in principle, but I think in practice this would be dead code 100% of the time. Might as well omit it then :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56116/new/ https://reviews.llvm.org/D56116 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56116: [gn build] Make `ninja check-clang` also run Clang's unit tests
thakis updated this revision to Diff 179744. thakis marked 2 inline comments as done. thakis added a comment. comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56116/new/ https://reviews.llvm.org/D56116 Files: llvm/utils/gn/build/BUILDCONFIG.gn llvm/utils/gn/build/sync_source_lists_from_cmake.py llvm/utils/gn/secondary/clang/lib/ASTMatchers/Dynamic/BUILD.gn llvm/utils/gn/secondary/clang/test/BUILD.gn llvm/utils/gn/secondary/clang/unittests/AST/BUILD.gn llvm/utils/gn/secondary/clang/unittests/ASTMatchers/BUILD.gn llvm/utils/gn/secondary/clang/unittests/ASTMatchers/Dynamic/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Analysis/BUILD.gn llvm/utils/gn/secondary/clang/unittests/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Basic/BUILD.gn llvm/utils/gn/secondary/clang/unittests/CodeGen/BUILD.gn llvm/utils/gn/secondary/clang/unittests/CrossTU/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Driver/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Frontend/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Index/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Lex/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Rename/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Rewrite/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Sema/BUILD.gn llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn llvm/utils/gn/secondary/clang/unittests/Tooling/BUILD.gn llvm/utils/gn/secondary/clang/unittests/libclang/BUILD.gn llvm/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn llvm/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni Index: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni === --- /dev/null +++ llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni @@ -0,0 +1,44 @@ +# This file defines a template for adding a unittest binary. +# +# It's a thin wrapper around GN's built-in executable() target type and +# accepts the same parameters. +# +# Example use: +# +# unittest("FormatTest") { +# sources = [ ... ] +# ... +# } + +template("unittest") { + executable(target_name) { +# Foward everything (configs, sources, deps, ...). +forward_variables_from(invoker, "*") +assert(!defined(invoker.output_dir), "cannot set unittest output_dir") +assert(!defined(invoker.testonly), "cannot set unittest testonly") + +# Common settings for all unit tests. +# Unit test binaries shouldn't go right in out/gn/bin, for two reasons: +# 1. That's where production binaries go. +# 2. The CMake build doesn't put the unit tests of all projects (clang, +#lld,...) in one directory, so it's not guaranteed that there won't +#be name collisions between test binaries from separate projects. +# Each lit suite takes an foo_obj_root parameter and puts temporary files +# for lit tests at foo_obj_root/test and looks for unit test binaries +# below foo_obj_root/unittests. As long as the BUILD.gn files processing +# the lit.site.cfg.py.in files match the output dir here, it doesn't +# matter all that much where the unit test binaries go, with the weak +# constraints that test binaries of different projects should go in +# different folders, and that it's not too difficult to manually +# run the unit test binary if necessary. Using target_out_dir here +# means that //clang/unittests/Format gets its binary in +# out/gn/obj/clang/unittests/Format/FormatTests, which seems fine. +output_dir = target_out_dir +deps += [ "//llvm/utils/unittest/UnitTestMain" ] +testonly = true + } +} + +set_defaults("unittest") { + configs = shared_binary_target_configs +} Index: llvm/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn === --- llvm/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn +++ llvm/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn @@ -1,13 +1,11 @@ -executable("MachOTests") { - # test/Unit/lit.cfg expects unittests in LLD_BINARY_DIR/unittest - output_dir = target_out_dir +import("//llvm/utils/unittest/unittest.gni") +unittest("MachOTests") { configs += [ "//llvm/utils/gn/build:lld_code" ] deps = [ "//lld/lib/Driver", "//lld/lib/ReaderWriter/MachO", "//lld/lib/ReaderWriter/YAML", -"//llvm/utils/unittest/UnitTestMain", ] sources = [ "MachONormalizedFileBinaryReaderTests.cpp", @@ -15,5 +13,4 @@ "MachONormalizedFileToAtomsTests.cpp", "MachONormalizedFileYAMLTests.cpp", ] - testonly = true } Index: llvm/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn === --- llvm/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn +++ llvm/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn @@ -1,15 +1,12 @@ -
[PATCH] D56116: [gn build] Make `ninja check-clang` also run Clang's unit tests
phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56116/new/ https://reviews.llvm.org/D56116 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56116: [gn build] Make `ninja check-clang` also run Clang's unit tests
This revision was automatically updated to reflect the committed changes. Closed by commit rL350171: [gn build] Make `ninja check-clang` also run Clang's unit tests (authored by nico, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D56116?vs=179744&id=179745#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56116/new/ https://reviews.llvm.org/D56116 Files: llvm/trunk/utils/gn/build/BUILDCONFIG.gn llvm/trunk/utils/gn/build/sync_source_lists_from_cmake.py llvm/trunk/utils/gn/secondary/clang/lib/ASTMatchers/Dynamic/BUILD.gn llvm/trunk/utils/gn/secondary/clang/test/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/AST/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/ASTMatchers/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/ASTMatchers/Dynamic/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Analysis/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Basic/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/CodeGen/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/CrossTU/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Driver/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Format/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Frontend/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Index/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Lex/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Rename/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Rewrite/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Sema/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/Tooling/BUILD.gn llvm/trunk/utils/gn/secondary/clang/unittests/libclang/BUILD.gn llvm/trunk/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn llvm/trunk/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn llvm/trunk/utils/gn/secondary/llvm/utils/unittest/unittest.gni Index: llvm/trunk/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn === --- llvm/trunk/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn +++ llvm/trunk/utils/gn/secondary/lld/unittests/DriverTests/BUILD.gn @@ -1,15 +1,12 @@ -executable("DriverTests") { - # test/Unit/lit.cfg expects unittests in LLD_BINARY_DIR/unittest - output_dir = target_out_dir +import("//llvm/utils/unittest/unittest.gni") +unittest("DriverTests") { configs += [ "//llvm/utils/gn/build:lld_code" ] deps = [ "//lld/lib/Driver", "//lld/lib/ReaderWriter/MachO", -"//llvm/utils/unittest/UnitTestMain", ] sources = [ "DarwinLdDriverTest.cpp", ] - testonly = true } Index: llvm/trunk/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn === --- llvm/trunk/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn +++ llvm/trunk/utils/gn/secondary/lld/unittests/MachOTests/BUILD.gn @@ -1,13 +1,11 @@ -executable("MachOTests") { - # test/Unit/lit.cfg expects unittests in LLD_BINARY_DIR/unittest - output_dir = target_out_dir +import("//llvm/utils/unittest/unittest.gni") +unittest("MachOTests") { configs += [ "//llvm/utils/gn/build:lld_code" ] deps = [ "//lld/lib/Driver", "//lld/lib/ReaderWriter/MachO", "//lld/lib/ReaderWriter/YAML", -"//llvm/utils/unittest/UnitTestMain", ] sources = [ "MachONormalizedFileBinaryReaderTests.cpp", @@ -15,5 +13,4 @@ "MachONormalizedFileToAtomsTests.cpp", "MachONormalizedFileYAMLTests.cpp", ] - testonly = true } Index: llvm/trunk/utils/gn/secondary/llvm/utils/unittest/unittest.gni === --- llvm/trunk/utils/gn/secondary/llvm/utils/unittest/unittest.gni +++ llvm/trunk/utils/gn/secondary/llvm/utils/unittest/unittest.gni @@ -0,0 +1,44 @@ +# This file defines a template for adding a unittest binary. +# +# It's a thin wrapper around GN's built-in executable() target type and +# accepts the same parameters. +# +# Example use: +# +# unittest("FormatTest") { +# sources = [ ... ] +# ... +# } + +template("unittest") { + executable(target_name) { +# Foward everything (configs, sources, deps, ...). +forward_variables_from(invoker, "*") +assert(!defined(invoker.output_dir), "cannot set unittest output_dir") +assert(!defined(invoker.testonly), "cannot set unittest testonly") + +# Common settings for all unit tests. +# Unit test binaries shouldn't go right in out/gn/bin, for two reasons: +# 1. That's where production binaries go. +# 2. The CMake build doesn't put the unit tests of all projects (clang, +#lld,...) in one directory, so it's not guaranteed that there won't +#be name collisions between test binaries from separat
[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors
dvyukov added a comment. This needs some tests, at least to exercise interceptors code once. Comment at: lib/esan/esan_interceptors.cpp:90 } while (false) +#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file) \ + do { \ The idea behind defining a no-op version in sanitizer_common_interceptors.inc was exactly that tools that are not interested in it does not need to be changed. So please remove this. Comment at: lib/tsan/rtl/tsan_interceptors.cc:2259 +#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file) \ + if (file) { \ An alternative would be to pass NULL to COMMON_INTERCEPTOR_FILE_OPEN and then do: if (path) Acquire(thr, pc, File2addr(path)) Just to not multiply entities. But neither approach looks strictly better to me, so I am not too strong about it. Repository: rCRT Compiler Runtime CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56157/new/ https://reviews.llvm.org/D56157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits