[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

rupprecht added a comment. BTW, seems like these docs have tests that weren't updated. Fixed with https://reviews.llvm.org/rL341002, but please take a look if that's not the right fix. Repository: rL LLVM https://reviews.llvm.org/D51190 ___ cfe

[PATCH] D49724: [VFS] Cleanups to VFS interfaces.

rupprecht added a comment. Looks like this caused some test failures (http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/14441/steps/ninja%20check%201), e.g.: [ RUN ] GoToInclude.All /home/buildslave/buildslave/clang-cmake-aarch64-quick/llvm/tools/clang/tools/extra/uni

[PATCH] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

rupprecht created this revision. rupprecht added reviewers: ldionne, rsmith. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, christof. [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4 r338122 changed the linkage of some methods which revealed an existi

[PATCH] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

rupprecht added a comment. In https://reviews.llvm.org/D49927#1178659, @ldionne wrote: > Just to make sure I understand properly: this means we will use newlib's > implementation of `iswcntrl_l` & friends instead of our own emulation (which > is an ODR violation currently going unnoticed)? And

[PATCH] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

This revision was automatically updated to reflect the committed changes. Closed by commit rL338157: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4 (authored by rupprecht, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.o

[PATCH] D53066: [Driver] Use forward slashes in most linker arguments

rupprecht resigned from this revision. rupprecht added a comment. I'm not familiar with this code or anything windows related, and it looks like @rnk is, so I'll remove myself and let him approve Repository: rL LLVM https://reviews.llvm.org/D53066 _

[PATCH] D56728: Regression test case for r350891 [Correct the source range returned from preprocessor callbacks]

rupprecht created this revision. rupprecht added a reviewer: aaron.ballman. Herald added subscribers: cfe-commits, jsji, kbarton, nemanjai. r350891 caused some internal failures that this test case exposes. Repository: rC Clang https://reviews.llvm.org/D56728 Files: unittests/Lex/PPCallbac

[PATCH] D56728: Regression test case for r350891 [Correct the source range returned from preprocessor callbacks]

rupprecht abandoned this revision. rupprecht added a comment. Looks like this test case was submitted as part of rL351209 . Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56728/new/ https://reviews.llvm.org/D56728 ___

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

rupprecht planned changes to this revision. rupprecht added a comment. I'll refactor `getLLVMStyle()` to `getLLVMStyle(FormatStyle::LanguageKind Language)` to support this change first. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55964/new/ https://reviews.llv

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

rupprecht added a comment. ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56943/new/ https://reviews.llvm.org/D56943 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/

[PATCH] D53952: [clang-format] Support breaking consecutive string literals for TableGen

rupprecht created this revision. rupprecht added a reviewer: krasimir. Herald added subscribers: cfe-commits, mgorny. Herald added a reviewer: alexshap. clang-format can get confused by string literals in TableGen -- e.g. try `clang-format tools/llvm-objcopy/ObjcopyOpts.td`; the multiline string

[PATCH] D53952: [clang-format] Support breaking consecutive string literals for TableGen

rupprecht added a comment. @djasper -- ping Repository: rC Clang https://reviews.llvm.org/D53952 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53952: [clang-format] Support breaking consecutive string literals for TableGen

This revision was automatically updated to reflect the committed changes. Closed by commit rC346687: [clang-format] Support breaking consecutive string literals for TableGen (authored by rupprecht, committed by ). Changed prior to commit: https://reviews.llvm.org/D53952?vs=172029&id=173711#toc

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

rupprecht updated this revision to Diff 188584. rupprecht marked 2 inline comments as done. rupprecht added a comment. Revert getLLVMStyle(LK_Cpp) fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56943/new/ https://reviews.llvm.org/D56943 Files

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

rupprecht added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:193 RawStringFormat.Language, &PredefinedStyle)) { -PredefinedStyle = getLLVMStyle(); +PredefinedStyle = getLLVMStyle(FormatStyle::LK_Cpp);

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

This revision was automatically updated to reflect the committed changes. Closed by commit rL355123: [clang-format][NFC] Allow getLLVMStyle() to take a language (authored by rupprecht, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

rupprecht updated this revision to Diff 188765. rupprecht added a comment. Rebase after NFC commit rC355123 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55964/new/ https://reviews.llvm.org/D55964 Files: clang

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

This revision was automatically updated to reflect the committed changes. Closed by commit rC355158: [clang-format][TableGen] Don't add spaces around items in square braces. (authored by rupprecht, committed by ). Changed prior to commit: https://reviews.llvm.org/D55964?vs=188765&id=188807#toc

[PATCH] D59557: Fix CodeGen/arm64-microsoft-status-reg.cpp test

rupprecht created this revision. rupprecht added reviewers: arsenm, MatzeB. Herald added subscribers: cfe-commits, kristof.beyls, javed.absar, wdng. Herald added a project: clang. This test is failing after r356499. Update the register selection used in the test. Repository: rG LLVM Github Mo

[PATCH] D59557: Fix CodeGen/arm64-microsoft-status-reg.cpp test

This revision was automatically updated to reflect the committed changes. Closed by commit rC356517: Fix CodeGen/arm64-microsoft-status-reg.cpp test (authored by rupprecht, committed by ). Changed prior to commit: https://reviews.llvm.org/D59557?vs=191379&id=191390#toc Repository: rC Clang

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

rupprecht added inline comments. Comment at: cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp:58 + std::vector Args = { + "-fopenmp", + }; Looks like this test fails when the default is not libomp, e.g. DCLANG_DEFAULT_OPENMP_RUNTIME=libgomp Using -fopen

[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

rupprecht created this revision. rupprecht added a reviewer: lebedev.ri. Herald added subscribers: cfe-commits, jdoerfert, guansong. Herald added a project: clang. rL356570 introduced a test which only passes with the default openmp library, libomp, and fails w

[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

rupprecht added a comment. In D59609#1436975 , @lebedev.ri wrote: > Interesting. > What happens if libomp is not being built? > What happens if libomp is not present? I'm far from an omp expert (only running across this because some tests failed), but

[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

This revision was automatically updated to reflect the committed changes. Closed by commit rC356614: [clang][OpenMP] Fix build when using libgomp (authored by rupprecht, committed by ). Changed prior to commit: https://reviews.llvm.org/D59609?vs=191567&id=191577#toc Repository: rC Clang CHA

[PATCH] D60974: Clang IFSO driver action.

rupprecht added a comment. I didn't follow the technical details, but I don't see anything wrong with moving forward on this patch. I think this seems like an interesting idea worth experimenting with. In D60974#1488563 , @jakehehrlich wrote: > > Jake,

[PATCH] D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr

rupprecht added a comment. In https://reviews.llvm.org/D52398#1255290, @aaronpuchert wrote: > @hokein Please have a look at https://reviews.llvm.org/D52888, maybe you can > try it out already. The problem was that `?:` expressions are considered a > branch point and when merging both branches t

[PATCH] D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr

rupprecht added a comment. In https://reviews.llvm.org/D52398#1257133, @aaronpuchert wrote: > In https://reviews.llvm.org/D52398#1257092, @rupprecht wrote: > > > I patched the proposed fix-forward and it seems to have things building > > again. Is there an ETA on landing that? If it's going to t

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

rupprecht added a comment. Herald added a project: clang. ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56943/new/ https://reviews.llvm.org/D56943 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

rupprecht added a comment. In D56943#1388314 , @kristina wrote: > The patch itself looks sound. However given that you have a specific use case > in mind (TableGen files) could you provide supplementary coverage for that > specific use case (unit tests f

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

rupprecht updated this revision to Diff 185787. rupprecht added a comment. Herald added a subscriber: arphaman. Herald added a project: clang. - Rebased w/ D56943 patched in so we can override just TableGen in getLLVMStyle() Repository: rG LLVM Github Monorep

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

rupprecht added a comment. In D56943#1388985 , @rupprecht wrote: > In D56943#1388314 , @kristina wrote: > > > The patch itself looks sound. However given that you have a specific use > > case in mind (TableGen file

[PATCH] D57896: Variable names rule

rupprecht added a comment. In D57896#1391925 , @hubert.reinterpretcast wrote: > In D57896#1391611 , @zturner wrote: > > > Is this actually any better? Whereas before we can’t differentiate type > > names and vari

[PATCH] D60974: Clang IFSO driver action.

rupprecht added a reviewer: rupprecht. rupprecht added a comment. Can you upload this patch with context? Either use arc or upload w/ -U9 I seem to have a lot of comments, but they're all nits -- my major concern being the `llvm_unreachable`s should be errors instead (i.e. should be trigger

[PATCH] D63625: [CodeGen][test] Use -fno-discard-value-names for better test support

rupprecht created this revision. rupprecht added reviewers: rnk, akhuang, aprantl. Herald added a project: clang. Herald added a subscriber: cfe-commits. Depending on how clang is built, it may discard the IR names and use names like `%2` instead of `%result.ptr`, causing tests that rely on the I

[PATCH] D63625: [CodeGen][test] Use -fno-discard-value-names for better test support

rupprecht updated this revision to Diff 205912. rupprecht added a comment. - Use filecheck variable matching instead of an explicit -fno-discard-value-names option Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63625/new/ https://reviews.llvm.org/D

[PATCH] D63625: [CodeGen][test] Use -fno-discard-value-names for better test support

rupprecht added a comment. Sounds good, changed to use variable matching instead. This passes w/ either `-fno-discard-value-names` or `-fdiscard-value-names` used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63625/new/ https://reviews.llvm.org/D

[PATCH] D63625: [CodeGen][test] Use -fno-discard-value-names for better test support

This revision was automatically updated to reflect the committed changes. Closed by commit rG73986707bd57: [CodeGen][test] Use FileCheck variable matchers for better test support (authored by rupprecht). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D61689: Change -gz and -Wa,--compress-debug-sections to use gABI compression (SHF_COMPRESSED)

rupprecht added a comment. So, while I think this is an //entirely// reasonable assumption in most cases, it's also not one that provides any kind of workaround for the few cases where it's not universally true. - As mentioned in the patch, this effectively changes the default from `-gz=zlib-g

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

rupprecht created this revision. rupprecht added reviewers: djasper, krasimir. Herald added a subscriber: cfe-commits. clang-formatting wants to add spaces around items in square braces, e.g. [1, 2] -> [ 1, 2 ]. Based on a quick check [1], it seems like most cases are using the [1, 2] format, so

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

rupprecht added a comment. Post-holiday ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55964/new/ https://reviews.llvm.org/D55964 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

rupprecht added a comment. In D55964#1351348 , @djasper wrote: > This seem to conceptually be a list of things rather than an array subscript, > though, right? Could we alternatively set SpacesInContainerLiterals to false > for LK_TableGen? That seems

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

rupprecht added a comment. Right, but `getGoogleStyle()` has the inferred language passed in (i.e. `getGoogleStyle(FormatStyle::LanguageKind Language)`. Whereas `getLLVMStyle()` takes no args and assumes C++. Would it be worthwhile to refactor getLLVMStyle to take in an optional language arg?

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

rupprecht updated this revision to Diff 180882. rupprecht added a comment. Move TableGen language check to where SpacesInContainerLiterals is checked Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55964/new/ https://reviews.llvm.org/D55964 Files: lib/Format/Tok

[PATCH] D48733: Introduce a separate preprocessor macro, _LIBUNWIND_USE_DLADDR, for directly controlling a dependency on dladdr(). This will allow us to use libunwind without adding a libdl dependency

rupprecht created this revision. rupprecht added a reviewer: saugustine. Herald added subscribers: cfe-commits, chrib, christof. Repository: rUNW libunwind https://reviews.llvm.org/D48733 Files: src/AddressSpace.hpp Index: src/AddressSpace.hpp ==

[PATCH] D48785: Add a blank line to docs/README.txt test commit access

rupprecht created this revision. Herald added subscribers: cfe-commits, christof. Repository: rUNW libunwind https://reviews.llvm.org/D48785 Files: docs/README.txt Index: docs/README.txt === --- docs/README.txt +++ docs/README

[PATCH] D48785: Add a blank line to docs/README.txt test commit access

This revision was automatically updated to reflect the committed changes. Closed by commit rL336006: Add a blank line to docs/README.txt test commit access (authored by rupprecht, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48785 File

[PATCH] D48787: Un-revert "Support for multiarch runtimes layout"This reverts https://reviews.llvm.org/rL336005, which was an accidental commit.

rupprecht created this revision. Herald added subscribers: cfe-commits, christof, mgorny. Repository: rUNW libunwind https://reviews.llvm.org/D48787 Files: CMakeLists.txt Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeL

[PATCH] D48733: Introduce a separate preprocessor macro, _LIBUNWIND_USE_DLADDR, for directly controlling a dependency on dladdr(). This will allow us to use libunwind without adding a libdl dependency

This revision was automatically updated to reflect the committed changes. Closed by commit rL336014: Introduce a separate preprocessor macro, _LIBUNWIND_USE_DLADDR, for directly… (authored by rupprecht, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://review

[PATCH] D49330: [compiler-rt] Include -lm when using compiler-rt, due to dependencies in some __div methods.

rupprecht created this revision. rupprecht added a reviewer: saugustine. Herald added subscribers: cfe-commits, aheejin, sbc100, dberris, srhines. Some methods in compiler-rt builtins have dependencies on libm. Therefore building with -rtlib=compiler-rt is completely broken if those symbols are u

[PATCH] D49330: [compiler-rt] Include -lm when using compiler-rt, due to dependencies in some __div methods.

rupprecht added a comment. Thanks Eli, I'll see how hard it is to remove those calls to logb/logbf (those seem to be the only ones causing link errors) and revert this change if that's possible. Repository: rC Clang https://reviews.llvm.org/D49330

[PATCH] D68669: [llvm-objdump][WIP] Make llvm-objdump -h compatible with GNU objdump.

rupprecht planned changes to this revision. rupprecht added a comment. Note: herald added reviewers, but this patch is just to provide context. I'll send the real patches for review in the coming days. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D68669: [llvm-objdump][WIP] Make llvm-objdump -h compatible with GNU objdump.

rupprecht created this revision. Herald added subscribers: llvm-commits, cfe-commits, seiya, arphaman, jakehehrlich, aheejin, arichardson, sbc100, emaste. Herald added a reviewer: espindola. Herald added a reviewer: alexshap. Herald added a reviewer: jhenderson. Herald added projects: clang, LLVM.

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

rupprecht added a comment. Just wanna say huge +1 for this patch. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67122/new/ https://reviews.llvm.org/D67122 ___ cfe-commits mailing list cfe-commi

[PATCH] D66490: [NewPM] Enable the New Pass Manager by Default in Clang

rupprecht added a comment. We already know that we don't want this enabled for tsan builds due to https://bugs.llvm.org/show_bug.cgi?id=42877, but I don't even know if anyone else will hit it (it's only when building one particular library). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

rupprecht added a comment. > Still think this looks good. Have you tried running this on the llvm test > suite, or some other interesting corpus? Would be curious to see any pre/post > patch numbers. I finally had time this morning to patch this in and give it a shot. (Sorry for the delay... i

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

rupprecht added a comment. > But TLDR, either the fix in https://github.com/google/filament/pull/1566 > is incorrect and the actually-bad code is elsewhere, > or you have some other unsanitized UB elsewhere. Could be both :S My money is on "both" :p I tested a random sample of a couple thousan

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

rupprecht added a comment. There's definitely a lot of new findings this creates, but it's hard to say exactly how many root causes there are due to the way test failures are (not) grouped well in the way I'm testing. So far they all seem like true positives, so this would be good to submit. Ho

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

rupprecht added a comment. In D71554#1789360 , @arichardson wrote: > Also handle -h/-v as short options. Does the adjusted test look okay? Sorry, didn't have time to take a second look before the holiday break -- yep, looks good, I didn't anticipate so

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

rupprecht added a comment. Thanks for the revert. In addition to the one eabi test, we saw widespread msan use-of-uninitialized-value errors from here: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/ARM/ARMTargetMachine.cpp#L229. I think it would explain the eabi test failure

[PATCH] D88640: [Format] Don't treat compound extension headers (foo.proto.h) as foo.cc main file.

rupprecht added a comment. It looks like this fix caused a different regression in not accepting `name..h` as the main header for `name..cc`, e.g.: $ cat /tmp/foo.bar.cc #include "a.h" #include "z.h" #include "foo.bar.h" $ clang-format /tmp/foo.bar.cc # Before #include "foo.bar.h

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

rupprecht added a comment. I'm seeing failures which I think are due to this patch -- I don't have a nice godbolt repro yet, but it's something like: foo.h: class Foo { public: static void DoStuff(); // Grabs mu_ private: static std::vector blah_ GUARDED_BY(mu_); static

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

rupprecht added a comment. In D84604#2363445 , @aaronpuchert wrote: > Pushed a fix in rGbbed8cfe80cd27d3a47d877c7608d9be4e487d97 > . For > now we just consider all static members as in

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

rupprecht added a comment. In D84604#2364568 , @aaronpuchert wrote: > In D84604#2363768 , @rupprecht wrote: > >> I applied D87194 locally and rebuilt the >> original source, and n

[PATCH] D87194: Thread safety analysis: Use access specifiers to decide about scope

rupprecht added a comment. In D87194#2369485 , @aaronpuchert wrote: > @rupprecht, maybe you can try it again? Some more interesting errors this time :) The ones I originally saw look correct now (i.e. it's flagging the things that are valid, but not th

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. FYI, I'm seeing what I think is a miscompile that bisects to this patch. Greatly simplified, the problematic snippet is this: struct Stats { int a; int b; int a_or_b; }; bool x = ... bool y = ... Stats stats; stats.a = x ? 1 : 0; stats.b =

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. The issue I'm seeing seems more directly caused by SLP vectorization, as it goes away with `-fno-slp-vectorize`. This patch merely unblocks that bad optimization AFAICT. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1011

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. In D101191#2782963 , @rupprecht wrote: > The issue I'm seeing seems more directly caused by SLP vectorization, as it > goes away with `-fno-slp-vectorize`. This patch merely unblocks that bad > optimization AFAICT. Filed as h

[PATCH] D97009: [CUDA] fix builtin constraints for PTX 7.2

rupprecht added a comment. This fixes a build error we're seeing, so I'd like to land this in a bit if that's OK. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97009/new/ https://reviews.llvm.org/D97009 ___

[PATCH] D97009: [CUDA] fix builtin constraints for PTX 7.2

This revision was automatically updated to reflect the committed changes. Closed by commit rG1a368ae3b78d: [CUDA] fix builtin constraints for PTX 7.2 (authored by tra, committed by rupprecht). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97009/new/

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. In D101191#2788596 , @spatel wrote: > In D101191#2783570 , @rupprecht > wrote: > >> In D101191#2782963 , @rupprecht >> wrote: >> >>> The issue

[PATCH] D104261: Thread safety analysis: Always warn when dropping locks on back edges

rupprecht added a comment. In D104261#2844636 , @aaronpuchert wrote: > In D104261#2841356 , @delesley > wrote: > >> since it's restricted to relockable managed locks, I'm not too worried... > > Not quite, it aff

[PATCH] D112732: [ASan] Process functions in Asan module pass

rupprecht added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1189 +MPM.addPass(ModuleAddressSanitizerPass( +CompileKernel, Recover, ModuleUseAfterScope, UseOdrIndicator, +DestructorKind, UseAfterScope, UseAfterReturn)); --

[PATCH] D112732: [ASan] Process functions in Asan module pass

rupprecht added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1189 +MPM.addPass(ModuleAddressSanitizerPass( +CompileKernel, Recover, ModuleUseAfterScope, UseOdrIndicator, +DestructorKind, UseAfterScope, UseAfterReturn)); --

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

rupprecht added a comment. I'm seeing a fair number of breakages from this patch (not really sure how many we truly have, I've hit ~5-10 so far in widely used libraries, but I suspect we have far more in the long tail). They're all valid (most are just adding missing thread safety annotations,

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

rupprecht added a comment. In D129755#3866213 , @aaronpuchert wrote: > Are you seeing warnings because of the different treatment of copy-elided > construction, or because we've started to consider `CXXConstructorCall`s > outside of the initializer of

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

rupprecht added a comment. In D129755#3869081 , @aaronpuchert wrote: > In D129755#3866887 , @rupprecht > wrote: > >> I might have a better answer in a day or two of how widespread this is >> beyond just the cor

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. Sorry for the delay, I was out on vacation for a bit. I have a repro for this new issue now: F25778542: repro.tar.gz $ CLANG=~/dev/clang ./repro.sh ++ dirname /tmp/repro/repro.sh + DIR=/tmp/repro + : /tmp/D136554 + rm -rf

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. Glad the test case made sense to you, it was convoluted to me :) Still seeing one more error, and it's not modules-related so I might be able to get it reduced today. Generally, it looks like this: struct Inner { Foo& foo; const std::unique_ptr<...> x = blah

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. In D136554#4013463 , @rupprecht wrote: > Glad the test case made sense to you, it was convoluted to me :) > > Still seeing one more error, and it's not modules-related so I might be able > to get it reduced today. Generally, it

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. All good now! The latest revision of this patch doesn't seem to break anything, unless I ran our tests wrong. From my perspective this is OK to reland now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https:

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. In D136554#4018870 , @rupprecht wrote: > All good now! The latest revision of this patch doesn't seem to break > anything, unless I ran our tests wrong. From my perspective this is OK to > reland now. ... and yep, I was holdi

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. I'm not sure what to make of the new failure when I try it out this time. Given a source like this: #include struct Options { std::function identity = [](bool x) -> bool { return x; }; }; struct Wrapper { explicit Wrapper(const Options& options =

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. I threw this at the "test everything" test (some millions of targets) and it found only one breakage, so this is very very close. Without further ado, here is this silly looking thing: File `blah.h`: #include #include template struct Base { virtual

[PATCH] D136554: Implement CWG2631

rupprecht accepted this revision. rupprecht added a comment. I still see one behavior change (actually it was there before, but I missed it in the test results), but as far as I can tell, it's a good one? If I reduce it too much, I get the warning with the baseline toolchain, so it's not erroneo

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. In D136554#4024628 , @rupprecht wrote: > I still see one behavior change (actually it was there before, but I missed > it in the test results), but as far as I can tell, it's a good one? If I > reduce it too much, I get the wa

[PATCH] D142449: [clang] Fix linking to LLVMTestingAnnotations in standalone build

rupprecht accepted this revision. rupprecht added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142449/new/ https://reviews.llvm.org/D142449 ___ cfe-commits mailing list cfe-

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

rupprecht added a comment. Hi, me again :) I ran into an interesting build breakage from this, I can't tell if it's a legitimate breakage based on reading P2036R3 and P2579R0 (mostly I'm not a language lawyer). struct StringLiteral { template StringLiteral(const char (&array)[N])

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

rupprecht added a comment. In D124351#4102679 , @aaron.ballman wrote: > In D124351#4102634 , @eaeltsin > wrote: > >>> Looks like we fail to enter the appropriate context somewhere (my guess is >>> that it might

[PATCH] D141175: [bazel] Split out Annotations from `TestingSupport`

rupprecht updated this revision to Diff 488366. rupprecht added a comment. Herald added subscribers: cfe-commits, kadircet, arphaman, hiraditya. Herald added projects: clang, clang-tools-extra. - Move annotations to a separate package entirely Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D141175: [bazel] Split out Annotations from `TestingSupport`

rupprecht added a comment. In D141175#4038017 , @GMNGeoffrey wrote: > It seems like the same logic would extend to the CMake build. Could we make > the same change there? The simplest (only?) way to do that is to have it literally in a separate direct

[PATCH] D141175: [test] Split out Annotations from `TestingSupport`

rupprecht updated this revision to Diff 488381. rupprecht added a comment. - Remove redundant comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141175/new/ https://reviews.llvm.org/D141175 Files: clang-tools-extra/clangd/unittests/Annotation

[PATCH] D141175: [test] Split out Annotations from `TestingSupport`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3432f4bf86e7: [test] Split out Annotations from `TestingSupport` (authored by rupprecht). Changed prior to commit: https://reviews.llvm.org/D14117

[PATCH] D153156: [Clang] CWG1473: do not err on the lack of space after operator""

rupprecht added a comment. In D153156#4599106 , @aaron.ballman wrote: > In D153156#4598988 , @rZhBoYao > wrote: > >> In D153156#4598915 , >> @steelannelida wrote: >> >>

[PATCH] D153156: [Clang] CWG1473: do not err on the lack of space after operator""

rupprecht added a comment. The other common breakage I'm seeing is code that hasn't yet migrated from the "PRI" format macros, of which there's an example in LLVM itself: https://github.com/llvm/llvm-project/blob/6a0e536ccfef1f7bd64ee4153b4efc0aeecf28d4/clang/test/SemaCXX/cxx11-compat.cpp#L38 e

[PATCH] D153156: [Clang] CWG1473: do not err on the lack of space after operator""

rupprecht added a comment. In D153156#4600270 , @rupprecht wrote: > of which there's an example in LLVM itself: > https://github.com/llvm/llvm-project/blob/6a0e536ccfef1f7bd64ee4153b4efc0aeecf28d4/clang/test/SemaCXX/cxx11-compat.cpp#L38 Sorry, I don't w

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13047 + F->setUsesFPIntrin(true); + printf("Enclosing function uses fp intrinsics\n"); +} Looks like this is leftover debugging? I'm seeing log spam compiling some files -- this m

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht marked an inline comment as done. rupprecht added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13047 + F->setUsesFPIntrin(true); + printf("Enclosing function uses fp intrinsics\n"); +} rupprecht wrote: > Looks like this is leftover

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444 +def warn_drv_experimental_fp_control_incomplete_opt : Warning< + "Support for floating point control option %0 is incomplete and experimental">, mibintc wrote

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444 +def warn_drv_experimental_fp_control_incomplete_opt : Warning< + "Support for floating point control option %0 is incomplete and experimental">, andrew.w.kayl

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

rupprecht accepted this revision. rupprecht added a comment. This revision is now accepted and ready to land. Looks good for D/U, but looks like --help and --version options are also supported as combined short args; do you mind adding those too while you're here? Thanks for the patch! =

  1   2   >