[PATCH] D69925: remove redundant LLVM version from version string when setting CLANG_VENDOR

srhines added a comment. In D69925#1736434 , @efriedma wrote: > I think the reason it's written this way is that CLANG_VERSION_STRING might > not correspond to an LLVM version? For example, Apple has its own versioning > scheme. Line 128 is already pr

[PATCH] D44815: [AArch64]: Add support for parsing rN registers.

srhines added a comment. Peter also requested that a test be added to make sure that rY was not accepted by the Clang assembler as a true synonym for xY. Comment at: lib/Basic/Targets/AArch64.cpp:320 +{{"r24"}, "x24"}, {{"r25"}, "x25"}, {{"r26"}, "x26"}, {{"r27"}, "x27"},

[PATCH] D44995: [Driver] Include the Android multiarch includes.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks for adding this support. Sorry about the delay in reviewing. Repository: rC Clang https://reviews.llvm.org/D44995 ___ cfe-commits mai

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

srhines added a comment. > They'd already have had a chance to deal with their code when this was a > warning-default-error without "ShowInSystemHeaders"? (or, if the yhaven't > picked up a new compiler often enough, and they go from "a warning we didn't > care about" to "warning-default-error-

[PATCH] D34114: [clang] A better format for unnecessary packed warning.

srhines added a comment. Richard's points are really valid here. I missed the aspect that packed always implies alignment 1, which does have an effect on the code in most of the cases listed here. I agree that the value of this warning is low, since the possibility of false-positive is quite hi

[PATCH] D35739: Fix LLVMgold plugin name/path for non-Linux.

srhines added a comment. Looks great. Thanks for fixing this up. It's hard to believe that there is nothing in Support that gives you the proper suffix for shared libraries. It would seem like this might be useful elsewhere, but I actually didn't find really any other place that wants to use th

[PATCH] D33561: [CMake] Add Android toolchain CMake cache files.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Awesome! https://reviews.llvm.org/D33561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

srhines added a comment. This looks better. It did lose context however, so some of the review is harder to read. Comment at: test/CodeGenCXX/warn-padded-packed.cpp:84 + +struct S16 { // expected-warning {{padding size of 'S16' with 2 bytes to alignment boundar}} expected-war

[PATCH] D42676: Add support for LLVM_REPOSITORY_STRING.

srhines created this revision. Herald added subscribers: cfe-commits, hintonda, mgorny. srhines added a reviewer: beanz. Without this, vendor Clang builds can occasionally pick up an arbitrary mirror location for the repository (at least on Android builds). This allows vendor builds to be more rep

[PATCH] D30015: [OpenMP] Add arch-specific directory to search path

srhines added inline comments. Comment at: include/clang/Driver/ToolChain.h:304 + // Returns /lib//. When -fopenmp is specified, + // this directory is added to the linker search path if it exists + const std::string getOpenMPLibPath() const; Add a period at

[PATCH] D30764: Disable unsigned integer sanitizer for basic_string::replace()

srhines added a comment. You probably want to remove the Change-Id section above in your description (or at least drop that when you finally submit this to libc++). https://reviews.llvm.org/D30764 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

srhines added a comment. Looks good, but I I want to make sure that someone else more familiar with this is ok with it too. Thanks. https://reviews.llvm.org/D27377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D33304: [clang-tidy] Add a new module Android and a new check for file descriptors.

srhines added inline comments. Comment at: clang-tidy/android/FileDescriptorCheck.cpp:65 + +diag(FlagArg->getLocStart(), "open(), openat(), and open64() " + "must include O_CLOEXEC in their flags argument.") Use "%0 " instead of the 3 function names,

[PATCH] D33304: [clang-tidy] Add a new module Android and a new check for file descriptors.

srhines added a comment. In https://reviews.llvm.org/D33304#758621, @joerg wrote: > I find the use of "must" at the very least inappropriate. If there was no use > case for not including it, it wouldn't be an option. There is also nothing > really Android-specific here beside maybe the open64 m

[PATCH] D116753: Default to -fno-math-errno for musl too

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Verified with the docs from musl. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116753/new/ https://reviews.llvm.org/D116753

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

srhines added a comment. In D96403#2639137 , @smeenai wrote: > In D96403#2638932 , @rprichard wrote: > >> In D96403#2638883 , @smeenai wrote: >> >>> With NDK r22, I only see

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

srhines added a comment. In D96403#2639180 , @srhines wrote: > In D96403#2639137 , @smeenai wrote: > >> In D96403#2638932 , @rprichard >> wrote: >> >>> In D96403#2638883

[PATCH] D99996: [Driver] Drop $DEFAULT_TRIPLE-$name as a fallback program name

srhines added a comment. Adding Dan as an FYI. While this doesn't impact Android platform or regular NDK users, I suppose that someone could have some esoteric build rules that are relying on this, but they should probably be more explicit about what they're running in those cases anyways. Re

<    1   2