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

This revision was automatically updated to reflect the committed changes. Closed by commit rG4baae166ce33: [pseudo] Fix pseudo-gen usage when cross-compiling (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126397/new/ https://re

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

smeenai added a comment. In D126397#3547060 , @thakis wrote: > Even after this, you still have to explicitly set > `-DCMAKE_SYSTEM_NAME=Darwin` when building clang for mac/arm on mac/intel. > Given that the host and target systems are both mac, that's a

[PATCH] D159293: [driver] Perform fallback target searches for stdlib

This revision was automatically updated to reflect the committed changes. Closed by commit rG1212d1b51125: [driver] Perform fallback target searches for stdlib (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159293/new/ https://

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

This revision was automatically updated to reflect the committed changes. Closed by commit rGb1e3cd1d7944: [driver] Conditionally include installed libc++ headers for Android (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159292

[PATCH] D158476: [driver] Search for compatible Android runtime directories

This revision was automatically updated to reflect the committed changes. Closed by commit rG58288c6c1214: [driver] Search for compatible Android runtime directories (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158476/new/ ht

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai added a comment. In D158476#4647652 , @MaskRay wrote: > nits Sorry, this came in right as I committed the diff. I pushed rG915ebb07dfc53486eccf0dc09b6411929a463e98 to addres

[PATCH] D153623: [clang][Sema] Add fixit for scoped enum format error

smeenai added a comment. Sorry for the delayed comment here. The fix-it is convenient, but is it the best suggestion? It'll end up suggesting truncating the enum value instead of using the proper format specifier in https://godbolt.org/z/xdhrefG95, for example. More insidiously, the `static_ca

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

smeenai added inline comments. Comment at: clang/test/Driver/cl-options.c:567 -// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs -// later on the command line, so it should win. Interestingly the cc1 arguments -// came out right, but had wrong seman

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

smeenai added a subscriber: mstorsjo. smeenai added inline comments. Comment at: clang/test/Driver/cl-options.c:567 -// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs -// later on the command line, so it should win. Interestingly the cc1 arguments -

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

smeenai added a comment. In D141918#4577930 , @jyknight wrote: > In D141918#4566838 , @smeenai wrote: > >> $ clang -std=c++20 -O2 -c crash.cpp >> cannot use musttail with unwindabort > > Thanks for the report. We

[PATCH] D158218: [CMake] Deprecate DEFAULT_SYSROOT and GCC_INSTALL_PREFIX

smeenai added a comment. Conceptually this seems like a great direction to go in. We should just leave it up for a while so that anyone using these can comment on any potential issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158218/new/ htt

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

smeenai added a comment. Herald added a subscriber: ekilmer. Herald added a project: libunwind. Herald added a reviewer: libunwind. I think we should land this. Clang has grown workarounds in the meantime, e.g. https://github.com/llvm/llvm-project/blob/4001ae175cbe351d496a6cd5481a554b346f706d/cla

[PATCH] D158475: [driver] Refactor getRuntimePaths. NFC

smeenai created this revision. smeenai added reviewers: collinbaker, thakis, phosek, MaskRay. Herald added a project: All. smeenai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This used to be getRuntimePath till https://reviews.llvm.org/

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai created this revision. smeenai added reviewers: collinbaker, thakis, MaskRay, phosek, danalbert, srhines. Herald added subscribers: danielkiss, kristof.beyls. Herald added a project: All. smeenai requested review of this revision. Herald added a project: clang. Herald added a subscriber: c

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai added a comment. In D158476#4605360 , @MaskRay wrote: >> // Android target triples contain a target version. If we don't have >> libraries for the exact target version, we should fall back to the next >> newest version or a versionless path, if

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai added a comment. In D158476#4608428 , @thakis wrote: > I think this is a useful feature, for the reasons mentioned on the thread. > > Since this is a superset of D115049 (... > right?), this should probably reve

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

smeenai added inline comments. Comment at: runtimes/CMakeLists.txt:167 +if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(CXX_TARGET_TRIPLE ${CMAKE_CXX_COMPILER} --target=${LLVM_RUNTIME_TRIPLE} -print-target-triple) ldionne wrote: > Is there any reas

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

smeenai added a comment. Thanks for the rebase! We're eager to try this for Meta's Android apps, where size is a priority and better `noexcept` codegen would be really helpful. I ran into a crash with coroutines though, which blocks me from fully evaluating the size difference made by these ch

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

smeenai added a comment. In D159292#4632389 , @rprichard wrote: > This change looks like an improvement to me. The NDK currently puts the > libc++ headers into the sysroot, but putting it in the usual toolchain > include location seems better. Are you

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai added a comment. Ping. I added the warning and release note. I haven't made search paths be dumped in `-v` mode because I thought it could be pretty noisy (especially since you'll be doing three searches, for resource dir libraries, libc++ per-target headers, and non-resource dir librar

[PATCH] D159486: Fix build error in CI.

smeenai added a comment. Generally it's preferred to either push bot failure fixes directly or revert the original commit (also without review), to get bots unblocked as soon as possible :) From what I can see, the test failure is because `dso_local` isn't emitted on Mac. That seems incidental

[PATCH] D159486: Fix build error in CI.

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. The `-triple x86_64-unknown-unknown` shouldn't be required after the regex change, but I'm fine either way. I'd recommend pushing this to fix the bots, and Aaron/Nico can do a post-commit re

[PATCH] D159486: Fix build error in CI.

smeenai added a comment. I don't think pre-merge testing covers Mac, only Linux and Windows (which is presumably how the original breakage got in). If the test passes for you locally I'd just push it and monitor the bots afterwards. (That isn't general advice, but in this specific case, since w

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

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

[PATCH] D158476: [driver] Search for compatible Android runtime directories

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

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

smeenai added a comment. In D159292#4646811 , @rprichard wrote: > In D159292#4646096 , @smeenai wrote: > >> Ping. > > I asked a couple of other people at Google to take a look at the patches. > Someone should get

[PATCH] D153989: [compiler-rt] Move crt into builtins

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Sorry, it's been a week :D I assume that crt doesn't need the builtins to be available for its configure (the way the rest of compiler-rt does)? If so, this LGTM. Comment

[PATCH] D153989: [compiler-rt] Move crt into builtins

smeenai added a comment. In D153989#4465907 , @phosek wrote: > In D153989#4465759 , @smeenai wrote: > >> Sorry, it's been a week :D I assume that crt doesn't need the builtins to be >> available for its configure

[PATCH] D155081: Specify the developer policy around links to external resources

smeenai added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:359 + If the patch fixes a bug in GitHub Issues, we encourage adding + "Fixes https://github.com/llvm/llvm-project/issues/12345"; to automate closing + the issue in GitHub. If the patch has been reviewed,

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai added a comment. In D158476#4609243 , @srhines wrote: > This approach LGTM, assuming that https://reviews.llvm.org/D140925 lands as > well to ensure that the triple does use `androideabi`, since it would prevent > custom build setups from needin

[PATCH] D158475: [driver] Refactor getRuntimePaths. NFC

smeenai updated this revision to Diff 553290. smeenai added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158475/new/ https://reviews.llvm.org/D158475 Files: clang/include/clang/Driver/ToolChain.h clang/lib/Driver/Driver.cpp

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai updated this revision to Diff 553291. smeenai added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158476/new/ https://reviews.llvm.org/D158476 Files: clang/include/clang/Driver/ToolChain.h clang/lib/Driver/ToolChain.c

[PATCH] D158798: [RFC] Android runtimes build demonstration

smeenai created this revision. smeenai added reviewers: danalbert, rprichard, srhines. Herald added a subscriber: danielkiss. Herald added a project: All. smeenai requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. This demons

[PATCH] D158798: [RFC] Android runtimes build demonstration

smeenai added a comment. The manual `androideabi` normalization is only needed when building the runtimes, not when using them: $ for i in {21..32}; do printf '%s ' $i; bin/clang --print-file-name=libclang_rt.builtins.a -target armv7-none-linux-androideabi$i; done 21 /data/users/smeenai/l

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai added a comment. D158798 demonstrates a working Android runtimes setup with per-target runtime directories once this change is in place. It'll be nicer once D140925 lands, but it's workable even without that. Reposi

[PATCH] D158475: [driver] Refactor getRuntimePaths. NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb6a1473f97d3: [driver] Refactor getRuntimePaths. NFC (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D158475: [driver] Refactor getRuntimePaths. NFC

smeenai added a comment. In D158475#4623471 , @glandium wrote: > This conflicts with D146664 It sounds like you want the same logic as D158476 to apply to the stdlib search as well as

[PATCH] D158475: [driver] Refactor getRuntimePaths. NFC

smeenai added a comment. In D158475#4623852 , @glandium wrote: > In D158475#4623842 , @smeenai wrote: > >> In D158475#4623471 , @glandium >> wrote: >> >>> This conflicts

[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

smeenai added a comment. In D152495#4628028 , @goncharov wrote: > there is a number of unused vaiables in conditional loops that are firing > now, I have submitted > https://github.com/llvm/llvm-project/commit/74f4daef0412be33002bd4e24a30cb47d0187ecf >

[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

smeenai added a comment. In D152495#4628877 , @hans wrote: > In D152495#4628870 , @goncharov > wrote: > >> due to this change we have a enourmous number of new warnings, on the other >> hand -Wunused-variable is

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

smeenai created this revision. smeenai added reviewers: danalbert, srhines, pcc, phosek, thakis, MaskRay, glandium. Herald added a subscriber: danielkiss. Herald added a project: All. smeenai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D159293: [driver] Perform fallback target searches for stdlib

smeenai created this revision. smeenai added reviewers: MaskRay, phosek, thakis, glandium. Herald added a subscriber: abrachet. Herald added a project: All. smeenai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Searching for target-specif

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai updated this revision to Diff 555082. smeenai added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158476/new/ https://reviews.llvm.org/D158476 Files: clang/include/clang/Driver/ToolChain.h clang/lib/Driver/ToolChain.c

[PATCH] D158475: [driver] Refactor getRuntimePaths. NFC

smeenai added a comment. In D158475#4626856 , @glandium wrote: > In D158475#4626636 , @smeenai wrote: > >> I'm halfway through an implementation of this, but I just realized that on >> Android, Clang never search

[PATCH] D159293: [driver] Perform fallback target searches for stdlib

smeenai updated this revision to Diff 555084. smeenai added a comment. Kick off pre-merge checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159293/new/ https://reviews.llvm.org/D159293 Files: clang/include/clang/Driver/ToolChain.h clang/lib

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai updated this revision to Diff 555086. smeenai added a comment. Kick off pre-merge checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158476/new/ https://reviews.llvm.org/D158476 Files: clang/include/clang/Driver/ToolChain.h clang/lib

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai added a reviewer: glandium. smeenai added a comment. @thakis I'm inclined to leave the fallback for a triple without any version in place for now, since it seems like Mozilla might be relying on it too, and we can remove it once everyone's moved over to the new fallback mechanism. What d

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

smeenai added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3108 +SmallString<128> TargetDir(Path); +llvm::sys::path::append(TargetDir, Target, "c++", Version); if (D.getVFS().exists(TargetDir)) rprichard wrote: > Will the `Target`

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai added a comment. In D158476#4632188 , @thakis wrote: > Works for me. How do you imagine the transition happening? Should we emit > some kind of warning if the old fallback is hit? That's a good call. I'll add that plus a release note. Reposito

[PATCH] D158476: [driver] Search for compatible Android runtime directories

smeenai updated this revision to Diff 555213. smeenai added a comment. Add warning and release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158476/new/ https://reviews.llvm.org/D158476 Files: clang/docs/ReleaseNotes.rst clang/include/cla

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

smeenai updated this revision to Diff 555246. smeenai added a comment. Rebase to avoid pre-existing CI failure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159292/new/ https://reviews.llvm.org/D159292 Files: clang/lib/Driver/ToolChains/Gnu.cpp

[PATCH] D159293: [driver] Perform fallback target searches for stdlib

smeenai updated this revision to Diff 555247. smeenai added a comment. Rebase to avoid pre-existing CI failure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159293/new/ https://reviews.llvm.org/D159293 Files: clang/include/clang/Driver/ToolChain

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

smeenai added a comment. We're seeing a CMake error after this: CMake Error at /data/users/smeenai/Source/tcdev/external/llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:368 (add_library): Error evaluating generator expression: $ Objects of target "RTGwpAsan.x86_

[PATCH] D138974: [CMake] Support injecting extra dependencies for perf-training

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Might be worth a mention in either https://llvm.org/docs/AdvancedBuilds.html#multi-stage-pgo, to make it a bit more discoverable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D138975: [perf-training] Support additional test suffixes

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138975/new/ https://reviews.llvm.org/D138975 _

[PATCH] D74996: [arcconfig] Delete subproject arcconfigs

smeenai created this revision. smeenai added reviewers: jyknight, phosek, rnk. Herald added a reviewer: bollu. Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, LLVM. Herald added subscribers: llvm-commits, openmp-commits, libcxx-commits, lldb-commits, Sanitizers, cfe-commits. >From

[PATCH] D74996: [arcconfig] Delete subproject arcconfigs

This revision was automatically updated to reflect the committed changes. Closed by commit rGe34ddc09f464: [arcconfig] Delete subproject arcconfigs (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74996/new/ https://reviews.llvm.

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

smeenai added a comment. Sorry for the late feedback here – we're in the process of testing with Clang 10 and noticed a behavior change caused by this commit. Consider the following piece of code: @protocol P @end @protocol Q @end @interface I @end @interface J : I @e

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

smeenai added a comment. Thanks for that explanation; that makes sense. Do you think that the `@interface J : I` would also ideally be an error then? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66831/new/ https://reviews.llvm.org/D66831 _

[PATCH] D71428: [clang] Move CLANG_BOOTSTRAP_CMAKE_ARGS above PASSTHROUGH_VARIABLES

smeenai added reviewers: beanz, phosek, compnerd, smeenai. smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71428/new/ https://reviews.llvm.

[PATCH] D71428: [clang] Move CLANG_BOOTSTRAP_CMAKE_ARGS above PASSTHROUGH_VARIABLES

This revision was automatically updated to reflect the committed changes. Closed by commit rG61c8ee6a2fa4: [clang] Move CLANG_BOOTSTRAP_CMAKE_ARGS above PASSTHROUGH_VARIABLES (authored by xinxinw1, committed by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D71507: [perf-training] Make training data location configurable

smeenai created this revision. smeenai added reviewers: beanz, compnerd, phosek, xiaobai. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. We may wish to keep the PGO training data outside the repository. Add a CMake variable to allow referencing an external lit testsu

[PATCH] D71507: [perf-training] Make training data location configurable

smeenai added a comment. I'm not sure where best to document this. We have https://llvm.org/docs/HowToBuildWithPGO.html, but that doesn't mention the build system's built-in PGO support as far as I can tell. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D71507: [perf-training] Make training data location configurable

This revision was automatically updated to reflect the committed changes. Closed by commit rG2c59c4ffb9c1: [perf-training] Make training data location configurable (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71507/new/ https

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

smeenai added a comment. I can commit this for you, but is it possible to write a test case? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73842/new/ https://reviews.llvm.org/D73842 ___ cfe-commits mai

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

smeenai added a comment. Is it worth adding a test that a function with an explicit xray-instrument attribute also has these other attributes applied? Comment at: clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp:8 +// RUN: -std=c++11 -triple x86_64-unknown-unknown -e

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

smeenai accepted this revision. smeenai added a comment. LGTM. I'll commit this for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73842/new/ https://reviews.llvm.org/D73842 ___ cfe-commits mailing

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

This revision was automatically updated to reflect the committed changes. Closed by commit rG14f870366a93: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs (authored by ianlevesque, committed by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

smeenai added a comment. I'm assuming this needs to be picked to 10.0? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73742/new/ https://reviews.llvm.org/D73742 ___ cfe-commits mailing list cfe-commits@

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

smeenai added a comment. In D73742#1871012 , @aganea wrote: > In D73742#1870961 , @smeenai wrote: > > > I'm assuming this needs to be picked to 10.0? > > > Yes! Is it up to the authors to integrate their patches to

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

smeenai added a comment. This also fixes https://bugs.llvm.org/show_bug.cgi?id=44886 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

smeenai added a subscriber: hans. smeenai added a comment. CC @hans for the 10.0 branch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

smeenai added a comment. I'm not sure why Phabricator is still showing Needs Review, but @rnk's approval should make this count as accepted. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe-c

[PATCH] D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode

smeenai added a comment. It looks like our behavior still differs from gcc in the case of a `static inline __attribute__((gnu_inline))` function: https://gcc.godbolt.org/z/cY9-HQ. We emit it and gcc doesn't. I don't think that combination makes a lot of sense, but I ran into it in some internal

[PATCH] D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode

smeenai added a comment. In D67414#1873445 , @efriedma wrote: > > https://gcc.godbolt.org/z/cY9-HQ > > gcc's behavior for your testcase makes no sense. We have to emit the > definition of a static function: it can't be defined in any other translation >

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

smeenai added a comment. Similar to the case @mstorsjo mentioned, this also causes the following code to fail now: struct S { unsigned size; }; template struct U { S s; unsigned size() { return s.size(); } }; This is obviously invalid, and I personally prefer get

[PATCH] D72873: [clang][xray] Add -fxray-ignore-loops option

This revision was automatically updated to reflect the committed changes. Closed by commit rG1d62be244108: [clang][xray] Add -fxray-ignore-loops option (authored by ianlevesque, committed by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D728

[PATCH] D72890: [xray] Allow instrumenting only function entry and/or only function exit

This revision was automatically updated to reflect the committed changes. Closed by commit rG97ba483026cd: [xray] Allow instrumenting only function entry and/or only function exit (authored by ianlevesque, committed by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

smeenai added a subscriber: plotfi. smeenai added a comment. CC @plotfi for the InterfaceStubs failures Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69825/new/ https://reviews.llvm.org/D69825 ___ cfe-

[PATCH] D73236: [clang-tidy] Add clang-tidy headers to clang distribution

smeenai added a comment. Can you upload the patch with context (`git diff -U` if you're using the web interface or via arcanist)? You'll want to add install targets for this, by adding a `COMPONENT` to the `install` command and referencing that in the target. See https://reviews.llvm.org/s

[PATCH] D73236: [clang-tidy] Add clang-tidy headers to clang distribution

smeenai added a comment. This LGTM, but I'll give the clang-tidy developers some time to weigh in. Comment at: clang-tools-extra/clang-tidy/CMakeLists.txt:106 +) + add_custom_target(clang-tidy-headers DEPENDS clang-headers) + set_target_properties(clang-tidy-headers PROPE

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

smeenai added a comment. Thanks Reid. I still need to look into why this is causing some existing tests to crash, but I'll also adjust the test. Repository: rC Clang https://reviews.llvm.org/D44619 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

smeenai added a comment. PURE_WINDOWS is all Windows except Cygwin. It's an LLVM thing, not a CMake thing. I don't know if setargv.obj is available on MinGW, and even if it is it's probably not gonna have the .obj extension, so this should probably be limited to MSVC. Repository: rC Clang

[PATCH] D44916: [Sema] Avoid crash for category implementation without interface

smeenai created this revision. smeenai added reviewers: doug.gregor, rjmccall, rsmith. When we have a category implementation without a corresponding interface (which is an error by itself), semantic checks for property accesses will attempt to access a null interface declaration and then segfault

[PATCH] D44916: [Sema] Avoid crash for category implementation without interface

This revision was automatically updated to reflect the committed changes. Closed by commit rL328654: [Sema] Avoid crash for category implementation without interface (authored by smeenai, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D449

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

smeenai added a comment. Finally got a chance to dig into some of the failures this patch is causing. Here's an example crasher (run with `clang -cc1 -triple i686-windows -emit-llvm`): struct Q { Q(int); ~Q(); }; struct Z {}; struct A { A(Q); }; struct B : Z, A { usi

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

smeenai planned changes to this revision. smeenai added a comment. This doesn't pass all tests right now, and I'll also need to change the test in accordance with the review comments. Repository: rC Clang https://reviews.llvm.org/D44619 ___ cfe-

[PATCH] D45523: [CodeGen] Handle __func__ inside __finally

smeenai created this revision. smeenai added reviewers: compnerd, majnemer, rnk. When we enter a __finally block, the CGF's CurCodeDecl will be null (because CodeGenFunction::StartFunction is given an empty GlobalDecl for a __finally block), and so the dyn_cast here will result in an assertion fai

[PATCH] D45523: [CodeGen] Handle __func__ inside __finally

smeenai added inline comments. Comment at: test/CodeGen/exceptions-seh-finally.c:281 +// CHECK-LABEL: define internal void @"?fin$0@0@finally_with_func@@"({{[^)]*}}) +// CHECK: call void @cleanup_with_func(i8* getelementptr inbounds ([18 x i8], [18 x i8]* @"??_C@_0BC@COAGBPGM@fi

[PATCH] D45523: [CodeGen] Handle __func__ inside __finally

This revision was automatically updated to reflect the committed changes. Closed by commit rL329836: [CodeGen] Handle __func__ inside __finally (authored by smeenai, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D45523 Files: cfe/trunk

[PATCH] D39317: Use -fuse-init-array if no gcc installation is found.

smeenai added a comment. > And lld currently silently mislinks inputs with .ctors sections Is there a bug for this? I see https://bugs.llvm.org/show_bug.cgi?id=31224, which links to https://reviews.llvm.org/D35509, but neither seems active. https://reviews.llvm.org/D39317 __

[PATCH] D39389: [MS] Allow access to ambiguous, inaccessible direct bases

smeenai added a comment. When you say "latest Windows SDK headers", do you mean SDK version 10.0.16299.15? https://reviews.llvm.org/D39389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D39149: [libc++] Prevent tautological comparisons

smeenai added a comment. Ping. Like I said, not a huge fan of this change, but also not a huge fan of running into clang diagnostics (especially since we build with `-Werror` downstream). https://reviews.llvm.org/D39149 ___ cfe-commits mailing lis

[PATCH] D39149: [libc++] Prevent tautological comparisons

smeenai planned changes to this revision. smeenai added a comment. In https://reviews.llvm.org/D39149#910858, @mclow.lists wrote: > If we have to go down this road, I'd prefer the approach used in > http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.

[PATCH] D39149: [libc++] Prevent tautological comparisons

smeenai added a comment. In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote: > I dislike this change fairly strongly. Agreed. Would you be happier with just disabling the diagnostics around the problematic parts? In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote: > 1. D

[PATCH] D39149: [libc++] Prevent tautological comparisons

smeenai added a comment. I'm thinking you could account for all possible type sizes in the existing (enabled by default) warning, and have a different warning for possibly tautological comparisons. E.g. if a `long` is being compared against `INT_MAX`, you know that's only tautological on some p

[PATCH] D39149: [libc++] Prevent tautological comparisons

smeenai added a comment. In https://reviews.llvm.org/D39149#910931, @lebedev.ri wrote: > - The fact that under *different* circumstances the comparison is not > tautological is not a false-positive. It's not technically a false positive, but my suspicion is that it'll make the warning a lot l

[PATCH] D39149: [libc++] Prevent tautological comparisons

smeenai added a comment. In https://reviews.llvm.org/D39149#911024, @rsmith wrote: > In https://reviews.llvm.org/D39149#910936, @smeenai wrote: > > > I'm thinking you could account for all possible type sizes in the existing > > (enabled by default) warning, and have a different warning for poss

[PATCH] D39149: [libc++] Prevent tautological comparisons

smeenai abandoned this revision. smeenai added a comment. I confirmed that these warnings go away with https://reviews.llvm.org/D39462 applied, and they reappear if I manually specify `-Wmaybe-tautological-constant-compare`. Thank you! https://reviews.llvm.org/D39149 ___

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

smeenai added a comment. This solves the issue in https://reviews.llvm.org/D39149, at least. Repository: rL LLVM https://reviews.llvm.org/D39462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

<    1   2   3   4   5   6   7   8   >