[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs added a comment. re-landed in https://github.com/llvm/llvm-project/commit/2c9cf9f4ddd01ae9eb47522266a6343104f9d0b5 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs updated this revision to Diff 249377. jroelofs added a comment. Don't fire on `#import`s. The error on Windows suggests it would normally do the "right" thing anyway: http://45.33.8.238/win/10088/step_8.txt

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs marked 6 inline comments as done. jroelofs added a comment. https://github.com/llvm/llvm-project/commit/52bbdad7d63fd060d102b3591b433d116a982255 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs updated this revision to Diff 249224. jroelofs added a comment. Add missing release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files:

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs updated this revision to Diff 249223. jroelofs added a comment. Implement review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files:

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs closed this revision. jroelofs marked 7 inline comments as done. jroelofs added a comment. Thanks for the review! https://github.com/llvm/llvm-project/commit/1e0669bfe05f0f48ee88152c4a1d581f484f8d67 Comment at:

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs updated this revision to Diff 249164. jroelofs added a comment. Fix comment, add docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files:

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs marked 3 inline comments as done. jroelofs added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:9 + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H +#define

[PATCH] D75489: [clang-tidy] Generalize HeaderFileExtensions.{h, cpp}. NFC

jroelofs closed this revision. jroelofs added a comment. https://github.com/llvm/llvm-project/commit/3486cc014b208df3897cf5656db0d0fdeae26d6b Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75489/new/ https://reviews.llvm.org/D75489

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

jroelofs closed this revision. jroelofs added a comment. https://github.com/llvm/llvm-project/commit/47caa69120e582bf1b795ec646f069c83b0e9456 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75621/new/ https://reviews.llvm.org/D75621

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

jroelofs marked an inline comment as done. jroelofs added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:44-46 bool parseFileExtensions(StringRef AllFileExtensions, - FileExtensionsSet , char Delimiter); +

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

jroelofs marked an inline comment as done. jroelofs added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:44-46 bool parseFileExtensions(StringRef AllFileExtensions, - FileExtensionsSet , char Delimiter); +

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

jroelofs updated this revision to Diff 248757. jroelofs added a comment. - Drop dead include. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75621/new/ https://reviews.llvm.org/D75621 Files:

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

jroelofs updated this revision to Diff 248755. jroelofs added a comment. - Don't spam the deprecation message. Move that to release notes. - Drop unnecessary `const`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75621/new/

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

jroelofs updated this revision to Diff 248357. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75621/new/ https://reviews.llvm.org/D75621 Files: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

jroelofs updated this revision to Diff 248245. jroelofs added a comment. Preserve backwards compatibility of ',' as a delimiter (for now). > The llvm::StringRef::split function can take multiple split characters, AFAIU, that's for multi-character delimiters, not multiple delimiters.

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs updated this revision to Diff 248246. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs updated this revision to Diff 248208. jroelofs marked an inline comment as not done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files:

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

jroelofs updated this revision to Diff 248206. jroelofs added a comment. Fix comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75621/new/ https://reviews.llvm.org/D75621 Files:

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs updated this revision to Diff 248203. jroelofs added a comment. Split out https://reviews.llvm.org/D75621 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files:

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

jroelofs created this revision. jroelofs added reviewers: njames93, aaron.ballman. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. https://reviews.llvm.org/D74669#inline-685657 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75621 Files:

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs marked 2 inline comments as done. jroelofs added a comment. In D74669#1902107 , @njames93 wrote: > Adding the parent revision means you don't need to have those changes in this > patch. IIUC, that is what I've already done:

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

jroelofs updated this revision to Diff 247764. jroelofs added a comment. implement review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files:

[PATCH] D75489: [clang-tidy] Generalize HeaderFileExtensions.{h, cpp}. NFC

jroelofs updated this revision to Diff 247762. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75489/new/ https://reviews.llvm.org/D75489 Files: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp

[PATCH] D75489: [clang-tidy] Generalize HeaderFileExtensions.{h, cpp}. NFC

jroelofs created this revision. jroelofs added reviewers: njames93, aaron.ballman. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. jroelofs added a child revision: D74669: [clang-tidy] New check: bugprone-suspicious-include. jroelofs marked an inline

[PATCH] D75489: [clang-tidy] Generalize HeaderFileExtensions.{h, cpp}. NFC

jroelofs marked an inline comment as done. jroelofs added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:14 #include "clang/Basic/SourceManager.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallSet.h" this

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs updated this revision to Diff 246604. jroelofs added a comment. Implement review feedback: - Re-purpose `HeaderFileExtensionsUtils.h` to support headers *and* sources. - Surround file extension in diagnostic with `''`s. I have these as two separate patches locally, but I don't know how

[PATCH] D28820: Warn when calling a non interrupt function from an interrupt on ARM

jroelofs added a comment. Herald added a subscriber: kristof.beyls. In D28820#945118 , @kc.austin2017 wrote: > In D28820#944865 , @jroelofs wrote: > > > https://bugs.llvm.org/show_bug.cgi?id=35527 > > > >

[PATCH] D74812: [Sema] Teach -Warm-interrupt-safety about func ptrs

jroelofs marked an inline comment as done. jroelofs added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:5931 + if (Caller->hasAttr()) { +const Decl *CalleeDecl = FDecl; +if (const auto *UO = dyn_cast(Fn->IgnoreParens())) { This

[PATCH] D74812: [Sema] Teach -Warm-interrupt-safety about func ptrs

jroelofs created this revision. jroelofs added reviewers: efriedma, weimingz, EricWF. Herald added subscribers: cfe-commits, kristof.beyls. Herald added a project: clang. Fixes: https://bugs.llvm.org/show_bug.cgi?id=35527 https://bugs.llvm.org/show_bug.cgi?id=35528 Repository: rG LLVM

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs updated this revision to Diff 245031. jroelofs added a comment. Make the diagnostics more accurate. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs marked an inline comment as done. jroelofs added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp:59 +Optional File = PP->LookupFile( +HashLoc /* FIXME: lies */, (FileName + RE).str(), IsAngled, nullptr,

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs updated this revision to Diff 244994. jroelofs added a comment. Implement review feedback re: DiagLoc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files:

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs marked an inline comment as done. jroelofs added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp:59 +Optional File = PP->LookupFile( +HashLoc /* FIXME: lies */, (FileName + RE).str(), IsAngled, nullptr,

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs updated this revision to Diff 244884. jroelofs added a comment. git format-patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs marked an inline comment as done. jroelofs added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h:26 +/// For the user-facing documentation see: +///

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs marked 4 inline comments as done and an inline comment as not done. jroelofs added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:62 + Check.diag(HashLoc, "suspicious #include") + <<

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs updated this revision to Diff 244883. jroelofs marked an inline comment as done. jroelofs added a comment. - Renamed to bugprone-suspicious-include - Removed FixIts in favor of `note:` suggestions, iff the suggested file exists on the header search path. - Removed unnecessary unused

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs marked an inline comment as done. jroelofs added a comment. In D74669#1878168 , @njames93 wrote: > I have a feeling this check should be called something along the lines of > bugprone-suspicous-include. That's a much better name, I like it.

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

jroelofs created this revision. jroelofs added a reviewer: aaron.ballman. Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai. Herald added a project: clang. Detects and fixes suspicious code like: `#include "foo.cpp"`. Inspired by:

[PATCH] D73904: [clang] stop baremetal driver to append .a to lib

jroelofs added a comment. +1, looks good with a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73904/new/ https://reviews.llvm.org/D73904 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61040: [Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions

jroelofs added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:122 + +/// \p Flag must be a flag accepted by the driver with its leading '-' removed, +// otherwise '-print-multi-lib' will not emit them correctly. Can we enforce this

[PATCH] D52530: [analyzer] scan-build: if --status-bugs is passed, don't forget about the exit status of the actual build

jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D52530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52151: Also manages clang-X as tool for scan-build

jroelofs added a comment. `$triple-clang`, also Repository: rC Clang https://reviews.llvm.org/D52151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52153: scan-build: Add support of the option --exclude like in scan-build-py

jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D52153 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51354: Fix the -print-multi-directory flag to print the selected multilib.

jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM. Thanks for fixing this! Repository: rC Clang https://reviews.llvm.org/D51354 ___ cfe-commits mailing list

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

jroelofs added a subscriber: efriedma. jroelofs added a comment. > the targets where you would want to use this can't run the libcxx testsuite anyway (because they don't have an operating system to run the test programs under). I used to run libcxx tests for an arm baremetal toolchain I was

[PATCH] D43159: Modernize: Use nullptr more.

jroelofs added a comment. Is it worth adding `-Werror=zero-as-null-pointer-constant` to the build? Repository: rCXX libc++ https://reviews.llvm.org/D43159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D42645: New simple Checker for mmap calls

jroelofs added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:399 + def MmapWriteExecChecker : Checker<"MmapWriteExec">, +HelpText<"Check if mmap() call is not both writable and executable">, +DescFile<"MmapWriteExecChecker.cpp">;

[PATCH] D42645: New simple Checker for mmap calls

jroelofs added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:1 +// MmapWriteExecChecker.cpp - Check for the prot argument +// jroelofs wrote: > Needs one of these at the top: > > ``` > //===- MmapWriteExecChecker.cpp - Check

[PATCH] D42645: New simple Checker for mmap calls

jroelofs added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:1 +// MmapWriteExecChecker.cpp - Check for the prot argument +// Needs one of these at the top: ``` //===- MmapWriteExecChecker.cpp - Check the mmap prot argument

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

jroelofs added a comment. > Should we go with current patch? You'll need a bunch of `UNSUPPORTED: has-no-random-device` or something like it, but other than that, I'm happy with the patch as-is. Repository: rCXX libc++ https://reviews.llvm.org/D41316

[PATCH] D41733: [Driver] Suggest correctly spelled driver options

jroelofs added inline comments. Comment at: lib/Driver/Driver.cpp:191 if (A->getOption().hasFlag(options::Unsupported)) { - Diag(diag::err_drv_unsupported_opt) << A->getAsString(Args); - ContainsError |= Diags.getDiagnosticLevel(diag::err_drv_unsupported_opt, -

[PATCH] D41733: [Driver] Suggest correctly spelled driver options

jroelofs added inline comments. Comment at: lib/Driver/Driver.cpp:191 if (A->getOption().hasFlag(options::Unsupported)) { - Diag(diag::err_drv_unsupported_opt) << A->getAsString(Args); - ContainsError |= Diags.getDiagnosticLevel(diag::err_drv_unsupported_opt, -

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

jroelofs added a comment. I say that because there are contexts where it is **absolutely critical** that https://xkcd.com/221/ not be the implementation of an RNG. Repository: rCXX libc++ https://reviews.llvm.org/D41316 ___ cfe-commits mailing

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

jroelofs added a comment. I'd much rather provide no implementation than one that lies. Broken builds are much safer than problems at runtime. Repository: rCXX libc++ https://reviews.llvm.org/D41316 ___ cfe-commits mailing list

[PATCH] D28820: Warn when calling a non interrupt function from an interrupt on ARM

jroelofs added a comment. In https://reviews.llvm.org/D28820#944365, @efriedma wrote: > > What is the best way to modify the code for this compiler change ? > > Currently, the "interrupt" attribute only has an effect on functions, not > function pointers, so your code won't work the way you

[PATCH] D40816: [libunwind] Use the correct variable name for target triple in lit

jroelofs added a comment. I think just remove it from them. Repository: rL LLVM https://reviews.llvm.org/D40816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40816: [libunwind] Use the correct variable name for target triple in lit

jroelofs added a comment. (i.e. you should delete that, since this is the last dependency on it AFAICT) Repository: rL LLVM https://reviews.llvm.org/D40816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40816: [libunwind] Use the correct variable name for target triple in lit

jroelofs added a comment. There's a: `set(TARGET_TRIPLE ...)` in each of these runtimes' CMakeLists.txt... Repository: rL LLVM https://reviews.llvm.org/D40816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40775: [libcxx] Add underscores to win32 locale headers.

jroelofs added a comment. looks fine to me, but this is the sort of thing that @EricWF usually wants the final say on. https://reviews.llvm.org/D40775 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40775: [libcxx] Add underscores to win32 locale headers.

jroelofs added inline comments. Comment at: src/support/win32/locale_win32.cpp:90 +__libcpp_locale_guard __current(__loc); va_list ap; +va_start( ap, __format ); __ap Comment at: src/support/win32/locale_win32.cpp:99 {

[PATCH] D40218: [Clang] Add __builtin_launder

jroelofs added inline comments. Comment at: include/clang/Basic/Builtins.def:480 BUILTIN(__builtin_thread_pointer, "v*", "nc") +BUILTIN(__builtin_launder, "v*v*", "nt") EricWF wrote: > jroelofs wrote: > > GCC's is type-generic: > > > > ``` > > #include > >

[PATCH] D40218: [Clang] Add __builtin_launder

jroelofs added inline comments. Comment at: include/clang/Basic/Builtins.def:480 BUILTIN(__builtin_thread_pointer, "v*", "nc") +BUILTIN(__builtin_launder, "v*v*", "nt") GCC's is type-generic: ``` #include bool is_type_generic() { int v; return

[PATCH] D39793: [asan] Remove semicolon after do {} while (0)

jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D39793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D39534: [libunwind] Add ifdefs around ELF specific parts of UnwindRegisters*.S for ARM

jroelofs added a comment. In https://reviews.llvm.org/D39534#915937, @mstorsjo wrote: > In https://reviews.llvm.org/D39534#915935, @jroelofs wrote: > > > In https://reviews.llvm.org/D39534#915920, @mstorsjo wrote: > > > > > In https://reviews.llvm.org/D39534#915916, @compnerd wrote: > > > > > >

[PATCH] D39534: [libunwind] Add ifdefs around ELF specific parts of UnwindRegisters*.S for ARM

jroelofs added a comment. In https://reviews.llvm.org/D39534#915920, @mstorsjo wrote: > In https://reviews.llvm.org/D39534#915916, @compnerd wrote: > > > Very well, if thats the current implementation in the AsmParser, thats > > reasonable. I don't think that the directive has anything to do

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

jroelofs added a comment. Needs a docs entry for the new flag (in libcxx's BuildingLibcxx.rst). Other than that, all the stuff I've asked you to add LGTM. I'd still appreciate @EricWF/@mclow's opinion on the meat of the functional change part of this though... I don't know all the implications

[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows

jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D38679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

jroelofs added a comment. In https://reviews.llvm.org/D38599#893990, @jroelofs wrote: > In https://reviews.llvm.org/D38599#893985, @danalbert wrote: > > > In https://reviews.llvm.org/D38599#893903, @jroelofs wrote: > > > > > That reminds me... this does need a testcase or two. > > > > > > Oh,

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

jroelofs added a comment. In https://reviews.llvm.org/D38599#893985, @danalbert wrote: > In https://reviews.llvm.org/D38599#893903, @jroelofs wrote: > > > That reminds me... this does need a testcase or two. > > > Oh, also, any test I add is going to fail, since the case I'm trying to > account

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

jroelofs added a comment. That reminds me... this does need a testcase or two. Repository: rL LLVM https://reviews.llvm.org/D38599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38711: typos in documentation?

jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D38711 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

jroelofs resigned from this revision. jroelofs added a comment. I'm not sure I'm the right person to review this. Repository: rL LLVM https://reviews.llvm.org/D38599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows

jroelofs added inline comments. Comment at: src/AddressSpace.hpp:521 unw_word_t *offset) { -#ifndef _LIBUNWIND_IS_BAREMETAL +#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) Dl_info dyldInfo; mstorsjo

[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows

jroelofs added inline comments. Comment at: src/AddressSpace.hpp:521 unw_word_t *offset) { -#ifndef _LIBUNWIND_IS_BAREMETAL +#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) Dl_info dyldInfo; Would it

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

jroelofs added a comment. I'm not sure it's better than writing the if/elseif/elseif/elseif out explicitly :/ Repository: rL LLVM https://reviews.llvm.org/D37629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37496: Fix validation of the -mthread-model flag in the Clang driver

jroelofs closed this revision. jroelofs added a comment. r312748 https://reviews.llvm.org/D37496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37493: Fix ARM bare metal driver to support atomics

jroelofs closed this revision. jroelofs added a comment. r312651 https://reviews.llvm.org/D37493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37493: Fix ARM bare metal driver to support atomics

jroelofs added a comment. Sure. I'll commit it for you once this build/test cycle is finished. https://reviews.llvm.org/D37493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35542: libcxxabi: Suppress LLVM_ENABLE_MODULES

jroelofs added a comment. Does the unwinder need this too? Repository: rL LLVM https://reviews.llvm.org/D35542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35426: [clang] Add abi-breaking-checks support to clang

jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D35426 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35038: [libunwind] Add a test harness

jroelofs added inline comments. Comment at: test/lit.cfg:36 +else: +lit_config.fatal('Could not find libcxx test directory for test imports' + ' in: %s' % libcxx_test_src_root) manojgupta wrote: > jroelofs wrote: > > manojgupta wrote: > >

[PATCH] D35038: [libunwind] Add a test harness

jroelofs added inline comments. Comment at: test/lit.cfg:36 +else: +lit_config.fatal('Could not find libcxx test directory for test imports' + ' in: %s' % libcxx_test_src_root) manojgupta wrote: > I do not have libcxx checked out since I

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

jroelofs added inline comments. Comment at: cmake/caches/Android-stage2.cmake:37 + set(RUNTIMES_${target}-linux-android_COMPILER_RT_INCLUDE_TESTS OFF CACHE BOOL "") + set(RUNTIMES_${target}-linux-android_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "") +

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

jroelofs added inline comments. Comment at: cmake/caches/Android-stage2.cmake:37 + set(RUNTIMES_${target}-linux-android_COMPILER_RT_INCLUDE_TESTS OFF CACHE BOOL "") + set(RUNTIMES_${target}-linux-android_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "") +

[PATCH] D35328: Reland "[Driver] Update Fuchsia driver path handling"

jroelofs accepted this revision. jroelofs added inline comments. This revision is now accepted and ready to land. Comment at: lib/Driver/ToolChains/Fuchsia.cpp:134 +static std::string computeTriple(llvm::Triple Triple) { + SmallString<64> T; `normalizeTriple`

[PATCH] D35038: [libunwind] Add a test harness

jroelofs closed this revision. jroelofs added a comment. r307266 https://reviews.llvm.org/D35038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35038: [libunwind] Add a test harness

jroelofs created this revision. Herald added a subscriber: mgorny. Mostly cargo-culted from libcxxabi, since the unwinder was forked from there in the first place. Might still be some cruft that's only applicable to libcxxabi in here, so let me know if you spot anything like that. I killed

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

jroelofs added a comment. In https://reviews.llvm.org/D33259#772184, @mgorny wrote: > This causes a test failure with non-standard CLANG_RESOURCE_DIR: https://reviews.llvm.org/D33877 (thanks for the patch) https://reviews.llvm.org/D33259 ___

[PATCH] D33877: [test] Fix baremetal test to allow any -resource-dir

jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM... thank you! Repository: rL LLVM https://reviews.llvm.org/D33877 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

jroelofs added a comment. what about the builtins? https://reviews.llvm.org/D33561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

jroelofs closed this revision. jroelofs added a comment. r303873 https://reviews.llvm.org/D33259 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

jroelofs updated this revision to Diff 100177. jroelofs marked an inline comment as done. jroelofs added a comment. Fix a cmake warning: Platform/baremetal to use this system, please send your config file to cm...@www.cmake.org so it can be added to cmake Your CMakeCache.txt file was copied

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

jroelofs marked 2 inline comments as done. jroelofs added inline comments. Comment at: lib/Driver/ToolChains/BareMetal.cpp:110 + SmallString<128> Dir(SysRoot); + llvm::sys::path::append(Dir, "include", "c++", "v1"); + return Dir.str(); compnerd wrote: > Is

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

jroelofs updated this revision to Diff 100131. jroelofs added a comment. implement feedback https://reviews.llvm.org/D33259 Files: cmake/caches/BaremetalARM.cmake lib/Driver/CMakeLists.txt lib/Driver/Driver.cpp lib/Driver/ToolChains/BareMetal.cpp lib/Driver/ToolChains/BareMetal.h

[PATCH] D33049: [libcxx] Support for Objective-C++ tests

jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D33049: [libcxx] Support for Objective-C++ tests

jroelofs added a comment. Would be good to test for the presence/absence of the flag, too: Comment at: test/libcxx/selftest/test.arc.pass.mm:10 +//===--===// + +int main() ``` #if

[PATCH] D32178: Delete unstable integration tests

jroelofs added a comment. @arphaman Those patches seem reasonable. https://reviews.llvm.org/D32178 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

jroelofs marked 2 inline comments as done. jroelofs added inline comments. Comment at: lib/Driver/ToolChains/BareMetal.cpp:107-108 +ArgStringList ) const { + CmdArgs.push_back("-lc++"); + CmdArgs.push_back("-lc++abi"); +

  1   2   >