[PATCH] D40740: [compiler-rt] Remove out of date comment

This revision was automatically updated to reflect the committed changes. Closed by commit rL319570: [compiler-rt] Remove out of date comment (authored by smeenai). Repository: rL LLVM https://reviews.llvm.org/D40740 Files: compiler-rt/trunk/CMakeLists.txt Index: compiler-rt/trunk/CMakeLi

[PATCH] D40774: [libcxx] Fix intrinsics for MSVC

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Are you actually using libc++ with cl? :) https://reviews.llvm.org/D40774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D40774: [libcxx] Fix intrinsics for MSVC

smeenai added a comment. In https://reviews.llvm.org/D40774#944749, @halyavin wrote: > In https://reviews.llvm.org/D40774#943993, @smeenai wrote: > > > Are you actually using libc++ with cl? :) > > > People mostly use it to compile on Windows but I heard someone launches the > result too. Since

[PATCH] D40774: [libcxx] Fix intrinsics for MSVC

smeenai added a subscriber: bcraig. smeenai added a comment. In https://reviews.llvm.org/D40774#945254, @halyavin wrote: > In https://reviews.llvm.org/D40774#944751, @smeenai wrote: > > > In https://reviews.llvm.org/D40774#944749, @halyavin wrote: > > > > > In https://reviews.llvm.org/D40774#9439

[PATCH] D40774: [libcxx] Fix intrinsics for MSVC

This revision was automatically updated to reflect the committed changes. Closed by commit rCXX319802: [libcxx] Fix intrinsics for MSVC (authored by smeenai). Repository: rCXX libc++ https://reviews.llvm.org/D40774 Files: include/algorithm Index: include/algorithm

[PATCH] D40680: [libc++] Create install-stripped targets

This revision was automatically updated to reflect the committed changes. Closed by commit rL319959: [libc++] Create install-stripped targets (authored by smeenai). Repository: rL LLVM https://reviews.llvm.org/D40680 Files: libcxx/trunk/include/CMakeLists.txt libcxx/trunk/lib/CMakeLists.t

[PATCH] D40929: Unblock Swift Calling Convention Mangling on Windows

smeenai added a comment. Patch is missing context. Comment at: lib/AST/MicrosoftMangle.cpp:2133 + llvm::errs() << "Unsupported CC for mangling: " << CC << ".\n"; case CC_Win64: case CC_X86_64SysV: You still need the default label, right? Reposi

[PATCH] D40685: [libunwind] Switch to add_llvm_install_targets

smeenai added a comment. The other runtime projects had the stripped installation targets added manually because they officially support being built completely standalone (i.e. without any LLVM components), so we can't rely on LLVM's CMake there. That's really unfortunate IMO, since it leads to

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

smeenai added a comment. Yeah, this makes sense to me if we factor out the commonalities. I agree that it's nicer to have a single option vs. having to specify every single native tool individually. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

smeenai added a comment. I just discovered this and was about to post on Discourse about it. Glad you're already on it :) I don't think this is quite correct though? It'll turn off unwind tables for AArch64 entirely, whereas we want to keep sync unwind tables. If we want to minimize changes to

[PATCH] D122965: Corrected A Command

smeenai added a comment. Thanks for the typo fix! Comment at: clang/test/CodeGen/c-unicode.c:5 int \uaccess = 0; // ALLOWED: "곎ss": // ALLOWED-NOT: "\uaccess": ps-19 wrote: > Comment Line No: 5 is // ALLOWED: "곎ss": i think "곎ss" in istake here, > according

[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

smeenai added a comment. Sorry for chiming in a bit late here. I'm working on getting a large internal codebase to compile after this change. Most of the issues are easy enough to deal with, but there's one Boost error I'd like to highlight: P8289 . (We might be

[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

smeenai added a comment. https://godbolt.org/z/axhbj3MPE is a simpler repro in Godbolt with Boost 1.79 (the latest version). I agree that it'll be tricky to manage expectations if we downgrade to a warning, but I also suspect that the first time many people will run into this new error is when

[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

smeenai added a comment. In D130058#3698213 , @shafik wrote: >> Given that we have a non-obvious (at least to me) issue in a widely used >> third-party library, would we consider giving users some way to opt out of >> this error, at least as a transitio

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

smeenai added a comment. Thank you! I'm worried that users might miss this if it's only in the release notes, and then we'd be in a similar situation again when we try converting it to an error. Maybe you could also include the bit about the diagnostic turning into error-only in the diagnostic

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

smeenai added a comment. In D131307#3704709 , @thakis wrote: > It's already an error, but it's a warning default-mapped to an error. You can > -Wno-error=name to downgrade it into a warning, but that requires an explicit > action. So people are unlikely

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

smeenai added a comment. All right, SGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131307/new/ https://reviews.llvm.org/D131307 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

smeenai added a comment. In D130058#3708209 , @ayermolo wrote: > +2 turning it in to warning. It's breaking our builds, due to boost. :( This was done by D131307 , which I'm hoping will land soon :) CHANGES SINCE LAST

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

smeenai added a comment. Should there be a test case to verify that the warning isn't triggered in a non-constexpr context anymore? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131528/new/ https://reviews.llvm.org/D131528 ___ cfe-commits ma

[PATCH] D124489: Deprecate LLVM_BUILD_EXTERNAL_COMPILER_RT

smeenai added a comment. I don't think many people are gonna notice a warning in the general spew of CMake configuration, tbh. Would it be too disruptive to make this an error instead and have a CMake option you can pass to get rid of the error? Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

smeenai added a comment. The following files have their line endings (when checked out on disk) changed from CRLF to LF by this patch. Seems harmless, but I just wanted to confirm that it was expected: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readabilit

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

smeenai added a comment. In D124563#3478623 , @aaronpuchert wrote: > So maybe we should simply remove > > # These test input files rely on two-byte Windows (CRLF) line endings. > clang-apply-replacements/Inputs/crlf/crlf.cpp text eol=crlf > clang-a

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

smeenai added a comment. If I check out this commit and then check out the previous commit, `clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp` and `clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected` become modified in my working directory; their l

[PATCH] D122931: [CMake][compiler-rt] Support for using in-tree libc++

smeenai requested changes to this revision. smeenai added a comment. This revision now requires changes to proceed. (clearing my queue while there are unaddressed comments) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122931/new/ https://reviews.l

[PATCH] D49587: [CMake] Support exporting runtimes using the CMake export

smeenai requested changes to this revision. smeenai added a comment. This revision now requires changes to proceed. Herald added a subscriber: abrachet. Herald added a reviewer: libc++. Herald added projects: libc++abi, libunwind, All. Herald added a reviewer: libc++abi. Herald added a reviewer: li

[PATCH] D124489: Deprecate LLVM_BUILD_EXTERNAL_COMPILER_RT

smeenai resigned from this revision. smeenai added a comment. LGTM after changing to an error with a warning downgrade. Resigning to clear my queue, plus the Apple folks should get the final accept here :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible

smeenai added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:456 Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile; + Options.MCOptions.EmitDwarfUnwind = CodeGenOpts.getEmitDwarfUnwind(); Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;

[PATCH] D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible

smeenai added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:456 Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile; + Options.MCOptions.EmitDwarfUnwind = CodeGenOpts.getEmitDwarfUnwind(); Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;

[PATCH] D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible

smeenai added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:456 Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile; + Options.MCOptions.EmitDwarfUnwind = CodeGenOpts.getEmitDwarfUnwind(); Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

smeenai added a comment. Herald added subscribers: pmatos, asb. Herald added a project: All. In D108905#2975712 , @rsmith wrote: > No decision as yet, but so far it looks very likely that we'll settle on the > rule that exceptions cannot have potentially

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

smeenai added a comment. In D108905#4654403 , @ChuanqiXu wrote: > In D108905#4654393 , @smeenai wrote: > >> In D108905#2975712 , @rsmith wrote: >> >>> No decision as yet,

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

smeenai added a comment. I'd vote for something like `-fassume-nothrow-exception-dtor` or `-fenforce-nothrow-exception-dtor` depending on if the patch will also implement the enforcement mentioned in https://github.com/llvm/llvm-project/issues/57375#issuecomment-1230590204, but we can bikeshed

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that an exception object' destructor is nothrow

smeenai added a comment. This looks great to me, thanks. @rjmccall should sign off on it though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108905/new/ https://reviews.llvm.org/D108905 ___ cfe-commits

[PATCH] D18174: Fix libcxx build on musl

smeenai added inline comments. Comment at: include/__mutex_base:51 #ifndef _LIBCPP_CXX03_LANG -constexpr mutex() = default; +#ifdef __GLIBC__ +constexpr EricWF wrote: > Limiting `constexpr` to GLIBC implementations only is incorrect; you want to > exclu

[PATCH] D36713: [libc++] Add a persistent way to disable availability

smeenai created this revision. Herald added a subscriber: mgorny. Add _LIBCPP_DISABLE_AVAILABILITY to the site config options, and add a cmake option LIBCXX_DISABLE_AVAILABILITY to control the site config. This is similar to other options which influence headers and therefore shold have some way t

[PATCH] D36719: [libc++] Add site config option for ABI macros

smeenai created this revision. Herald added a subscriber: mgorny. Some ABI macros affect headers, so it's nice to have a site config option for them. Add a LIBCXX_ABI_DEFINES cmake macro to allow specifying a list of ABI macros to define in the site config. The primary design constraint (as discu

[PATCH] D36720: [libc++] Prevent stale site configuration headers

smeenai created this revision. Herald added a subscriber: mgorny. If we define cmake macros that require a site config, and then undefine all such macros, a stale site config header will be left behind. Explicitly delete any generate site config if we don't need one to avoid this. https://review

[PATCH] D36719: [libc++] Add site config option for ABI macros

smeenai added a comment. There should probably be some documentation for this, but I couldn't think of the right place; the Using libc++ documentation only mentions the actual configuration macros, not their corresponding cmake defines. Any suggestions? https://reviews.llvm.org/D36719 _

[PATCH] D36713: [libc++] Add a persistent way to disable availability

smeenai added a comment. In https://reviews.llvm.org/D36713#841478, @mclow.lists wrote: > What's the use case here? What are you trying to accomplish? The `_LIBCPP_DISABLE_AVAILABILITY` macro already exists, but since it influences headers, it's a lot more useful to have it as a site config o

[PATCH] D36713: [libc++] Add a persistent way to disable availability

smeenai added a comment. @mclow.lists does this seem reasonable to you? https://reviews.llvm.org/D36713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36719: [libc++] Add site config option for ABI macros

smeenai added a comment. Ping. https://reviews.llvm.org/D36719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36713: [libc++] Add a persistent way to disable availability

smeenai added a comment. Ping. https://reviews.llvm.org/D36713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36720: [libc++] Prevent stale site configuration headers

smeenai added a comment. Ping. https://reviews.llvm.org/D36720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70764: build: reduce CMake handling for zlib

smeenai added a comment. In D70764#1803559 , @raj.khem wrote: > this is now in master, and I am seeing build failures in cross-building > clang, e.g. when building clang for arm on a x86_64 host. its resorting to > finding, libz from buildhost instead of

[PATCH] D69770: [APFloat] Add recoverable string parsing errors to APFloat

smeenai added inline comments. Herald added a subscriber: herhut. Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:3133 return TokError("invalid floating point literal"); - } else if (Value.convertFromString(IDVal, APFloat::rmNearestTiesToEven) == - APFloat::opI

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

smeenai added a comment. What was the conclusion for the comments about the priority level (100 vs. 101)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31413/new/ https://reviews.llvm.org/D31413 ___ cfe-

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

smeenai added a reviewer: compnerd. smeenai resigned from this revision. smeenai added a subscriber: compnerd. smeenai added a comment. I'm super excited to see this happen, but it's also well out of my review comfort zone, unfortunately. Adding @compnerd to take a look. Repository: rG LLVM G

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai added inline comments. Comment at: clang/cmake/modules/AddClang.cmake:121 + set(export_to_clangtargets EXPORT Clang${distribution}Targets) + set_property(GLOBAL PROPERTY CLANG${distribution}_HAS_EXPORTS True) endif() phosek wrot

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

smeenai added a comment. In D45639#2360619 , @smeenai wrote: > In D45639#2360267 , @ldionne wrote: > >> - AppleClang prefers the headers in the SDK, and the library in the SDK. >> (The headers are currently still s

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai created this revision. smeenai added reviewers: beanz, compnerd, phosek, tstellar. Herald added subscribers: llvm-commits, cfe-commits, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_ami

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai added a comment. In D89177#2325566 , @phosek wrote: > We've already considered introducing a similar mechanism so thank you for > working on this! There's one issue that I haven't figured out how to resolve > and I'd be interested in your thought

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai added a comment. Reading up on the Ninja multi-config generator a bit, you can define your own build types, so you should be able to create e.g. one build type for LTO and another for PIC. You'd still need some external logic to define that you use the LTO mode for executables and the P

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai updated this revision to Diff 298251. smeenai added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89177/new/ https://reviews.llvm.org/D89177 Files: clang/cmake/caches/MultiDistributionExample.cmake cl

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai added a subscriber: ldionne. smeenai added a comment. In D89177#2326875 , @phosek wrote: > In D89177#2326832 , @smeenai wrote: > >> In D89177#2325566 , @phosek wrote:

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai added a comment. In D89177#2332675 , @beanz wrote: > In D89177#2332627 , @ldionne wrote: > >> That isn't what I meant. It's entirely okay for the runtimes to be driven >> via `AddExternalProject` like the r

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai added a comment. Ping. Assuming everyone is okay with the direction, any comments on the change itself? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89177/new/ https://reviews.llvm.org/D89177 _

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

smeenai added a comment. In D45639#2360267 , @ldionne wrote: > - AppleClang prefers the headers in the SDK, and the library in the SDK. (The > headers are currently still shipped in the toolchain but they should be > ignored if you have a sufficiently re

[PATCH] D90517: [clangd] Account for vendor in version string

smeenai created this revision. smeenai added reviewers: sammccall, kadircet, hokein. Herald added subscribers: cfe-commits, usaxena95, arphaman. Herald added a project: clang. smeenai requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. The vendor will be prefixed t

[PATCH] D90516: [clang] Limit scope of CLANG_VENDOR definition

smeenai created this revision. smeenai added reviewers: beanz, compnerd, phosek, tstellar. Herald added subscribers: cfe-commits, dexonsmith, mgorny. Herald added a project: clang. smeenai requested review of this revision. It's only used by Version.cpp, so limit the definition to just that one fi

[PATCH] D90518: [clangd] Make tests depend on Clang

smeenai created this revision. smeenai added reviewers: jkorous, kadircet, sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, mgorny. Herald added a project: clang. smeenai requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. The clangd lit confi

[PATCH] D90518: [clangd] Make tests depend on Clang

smeenai abandoned this revision. smeenai added a comment. Cool, thanks for fixing it better! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90518/new/ https://reviews.llvm.org/D90518 ___ cfe-commits maili

[PATCH] D90528: [clangd] Fix check-clangd with no clang built

smeenai added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90528/new/ https://reviews.llvm.org/D90528 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D90516: [clang] Limit scope of CLANG_VENDOR definition

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4e4ab8e0152b: [clang] Limit scope of CLANG_VENDOR definition (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D90517: [clangd] Account for vendor in version string

This revision was automatically updated to reflect the committed changes. Closed by commit rG6bd01b8184de: [clangd] Account for vendor in version string (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90517/new/ https://reviews.

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89177/new/ https://reviews.llvm.org/D89177 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D89177: [cmake] Add support for multiple distributions

smeenai updated this revision to Diff 344666. smeenai added a comment. Add release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89177/new/ https://reviews.llvm.org/D89177 Files: clang/cmake/caches/MultiDistributionExample.cmake clang/cm

[PATCH] D89177: [cmake] Add support for multiple distributions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG56f7e5a822b4: [cmake] Add support for multiple distributions (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D95534: clang-cl: Invent a /winsysroot concept

smeenai added a comment. I saw this on LLVM Weekly, and I like it a lot :) Now if only Windows could case-correct their SDKs so we didn't need VFS overlays and symlinks for the linker... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95534/new/ ht

[PATCH] D95204: [lld-macho] Switch default to new Darwin port

smeenai added inline comments. Comment at: llvm/utils/gn/build/BUILD.gn:240 if (host_os == "mac") { - ldflags += [ "-fuse-ld=lld.darwinnew" ] + ldflags += [ "-fuse-ld=ld64.lld" ] } else { I'm pretty sure `-fuse-ld=ld64.lld` won't work ... the

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path

smeenai added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:494 + std::string Ret(P); + // When LLVM_DEFAULT_TARGET_TRIPLE uses Debian multiarch style + // "x86_64-linux-gnu" (no vendor part), use the unnormalized This is effectively reverting th

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

smeenai added a comment. In D110663#3029076 , @MaskRay wrote: > Prefer unnormalized over normalized (@smeenai) To be clear, I wasn't advocating for one ordering over the other, just pointing out the similarity to the prior state of things :) @phosek wou

[PATCH] D109506: [clangd] Print current request context along with the stack trace

smeenai added a comment. In D109506#3086183 , @thakis wrote: > This doesn't build on windows: http://45.33.8.238/win/47615/step_4.txt > > Please take a look and revert for now if it takes a while to fix. I pushed rGba94b8bdffb4

[PATCH] D93164: [AST] Add generator for source location introspection

smeenai added inline comments. Comment at: clang/lib/Tooling/CMakeLists.txt:29-30 +if (NOT Python3_EXECUTABLE +OR WIN32 +OR APPLE +OR GENERATOR_IS_MULTI_CONFIG I'm looking at this commit in the context of https://bugs.llvm.org/show_bug.cgi?id=52106.

[PATCH] D97817: [CMake] Rename check-clang-tools to check-clang-tools-extra

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM. If you care about eventually getting rid of the old `check-clang-tools` alias, you'd want to have a mailing list announcement/deprecation notices/etc. (I don't think leaving it around

[PATCH] D97817: [CMake] Rename check-clang-tools to check-clang-tools-extra

smeenai added inline comments. Comment at: clang-tools-extra/test/CMakeLists.txt:81 -add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests" +add_lit_testsuite(check-clang-tools-extra "Running the Clang tools extra' regression tests" ${CMAKE_C

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

smeenai added a comment. In D92808#2593331 , @ahatanak wrote: > In D92808#2593204 , @thakis wrote: > >> FYI, we're seeing test failures caused by this patch in iOS/arm64 builds >> when arc is used (simulator is fin

[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

smeenai created this revision. smeenai added reviewers: jkorous, arphaman, akyrtzi, gribozavr. smeenai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We've observed this test being significantly flaky on our Mac CI machines when we're runn

[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

smeenai updated this revision to Diff 328283. smeenai added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97878/new/ https://reviews.llvm.org/D97878 Files: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.

[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

smeenai added a comment. In D97878#2602077 , @plotfi wrote: > This makes sense to me. I approve. Can we move the 3/60 seconds number to a > const int value set somewhere higher up in the file as a global with a > comment explaining this as well? Updated

[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

This revision was automatically updated to reflect the committed changes. Closed by commit rG9a2a167b6ca7: [DirectoryWatcher] Increase timeout to make test less flaky (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97878/new/ ht

[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

smeenai added a comment. Hilariously enough, this breaks building compiler-rt itself inside LLVM's runtime builds setup for us. The runtimes build setup builds clang and then uses the just-built clang to build compiler-rt. That build fails to link since my just-built clang doesn't have compiler

[PATCH] D136664: [CMake] Support building crt with the bootstrapping build

smeenai added a comment. I might be missing it, but I don't see `crt` depending on `builtins` (or vice versa). If there is actually no dep, could we build them together in a single configure, instead of needing to add another one for crt? If there is a dep and I missed it, then this LGTM. Rep

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

smeenai added a comment. Was it intended that the warning generated here isn't silenced by `-w`, only by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and that `-Wno-error` doesn't downgrade the error? See https://godbolt.org/z/s9qPveTWG for an example. Repository: r

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

smeenai added a comment. In D131307#3726643 , @erichkeane wrote: > In D131307#3726631 , @smeenai wrote: > >> Was it intended that the warning generated here isn't silenced by `-w`, only >> by an explicit `-Wno-e

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

smeenai added a comment. In D131528#3723231 , @ayermolo wrote: > In D131528#3722395 , @shafik wrote: > >> In D131528#3719430 , @ayermolo >> wrote: >> >>> Was this and pre

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

smeenai added a comment. I confirmed that this fixes all remaining issues on our end. Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131874/new/ https://reviews.llvm.org/D131874 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

smeenai added a comment. This is breaking our build setup. It causes e.g. the Clang resource headers to be installed to `lib64/clang` instead of `lib/clang`. Given this and the other breakages, can we revert for now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D131963: [libc++] Add custom clang-tidy checks

smeenai added a comment. In D131963#3811811 , @ldionne wrote: > I'd be fine with this as-is if it works in the CI. > > IIUC, the current blocker for this is that the `ClangConfig.cmake` installed > by LLVM is not robust to the dev packages missing. If yo

[PATCH] D131963: [libc++] Add custom clang-tidy checks

smeenai added a comment. In D131963#3845092 , @philnik wrote: > In D131963#3831786 , @smeenai wrote: > >> In D131963#3811811 , @ldionne >> wrote: >> >>> I'd be fine with

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

smeenai added subscribers: yozhu, lanza, smeenai. smeenai added a comment. I'm investigating a size increase we observed after this change for Meta's Android apps. This increases the size of compiler-rt by 1.6 KB, which is small by itself, but then compiler-rt is statically linked into each SO,

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

smeenai added a comment. In D127812#4031101 , @ilinpv wrote: > In D127812#4030881 , @smeenai wrote: > >> We're not actually using multi-versioning anywhere, but we're still paying >> the size cost for it as a res

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

smeenai added a comment. In D127812#4031849 , @ilinpv wrote: > In D127812#4031313 , @smeenai wrote: > >> We can use `-mno-fmv` to avoid that dependency, right? We're interested in >> using that for our own code (

[PATCH] D140925: [CMake] Use Clang to infer the target triple

smeenai accepted this revision. smeenai added a comment. @tamas' suggestion would be a good change to make IMO, but I think it's outside the scope of this patch, and the patch as-is improves the status quo, so LGTM. Is there any way to share the normalization logic between the two locations, or

[PATCH] D136664: [CMake] Support building crt with the bootstrapping build

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. In D136664#3884796 , @phosek wrote: > In D136664#3882989 , @smeenai wrote: > >> I might be missing it, but

[PATCH] D96147: [SEMA] Added warn_decl_shadow support for structured bindings

smeenai added a comment. Herald added a project: All. This is very belated, sorry :) Our shadowing warnings have special behavior for lambdas and local variables: the warning for lambdas shadowing uncaptured local variables falls under `-Wshadow-uncaptured-local`, which isn't turned on for `-Ws

[PATCH] D125667: [pseudo] A basic implementation of compiling cxx grammar at build time.

smeenai added a comment. This breaks cross-compilation, because we're building `pseudo-gen` for the target but then trying to run it on the build machine. You'll need to do something like TableGen does (see https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/CrossCompile.cmake an

[PATCH] D125667: [pseudo] A basic implementation of compiling cxx grammar at build time.

smeenai added a comment. In D125667#3537655 , @sammccall wrote: > I'll see if I can fix it quickly, else will revert (there are a couple of > fixes stacked on top already, so revert isn't quite clean) Thank you! I'll have some time to look at this a bit

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

smeenai created this revision. smeenai added reviewers: hokein, sammccall. Herald added a subscriber: mgorny. Herald added a project: All. smeenai requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. Use the LLVM build s

[PATCH] D125667: [pseudo] A basic implementation of compiling cxx grammar at build time.

smeenai added a comment. In D125667#3537678 , @smeenai wrote: > In D125667#3537655 , @sammccall > wrote: > >> I'll see if I can fix it quickly, else will revert (there are a couple of >> fixes stacked on top alr

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

smeenai added a comment. In D126397#3537843 , @sammccall wrote: > Thank you! I have been banging my head against this, I'm not entirely sure > why this works and my version doesn't. Ah, sorry for the duplicated work :( Thanks for the review! Repositor

<    1   2   3   4   5   6   7   8   >