[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 153431. vsapsai added a comment. Try to exclude changes available in parent review from this review. https://reviews.llvm.org/D48342 Files: libcxx/include/memory libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ vsapsai wrote: > erik.pilkington wrote: > > vsapsai wrote: > > > erik.pilkington wrote: > > > > Shouldn't this be true_type? > > > I see thi

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai added a comment. In https://reviews.llvm.org/D48753#1147500, @Quuxplusone wrote: > I'm pretty sure that the C++03 standard doesn't permit the implementation to > call any `construct` method here, even if it wanted to. And this adds a > couple hundred LOC to get that (non-standard) behav

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

vsapsai created this revision. vsapsai added reviewers: bruno, rsmith. Herald added a subscriber: dexonsmith. Fixes a problem when we have multiple inclusion cycles and try to enumerate all possible ways to reach the max inclusion depth. rdar://problem/38871876 https://reviews.llvm.org/D48786

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added a comment. In https://reviews.llvm.org/D48342#1148751, @mclow.lists wrote: > I want to point out that this code (not -necessarily- this patch, but where > it lives) needs to be rewritten. > > There is no prohibition on users specializing `allocator_traits` for their > allocators,

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai planned changes to this revision. vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ vsapsai wrote: > vsapsai wrote: > > erik.pilkington wrote: > > > vsapsai wrote: > > > > erik.pilki

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai updated this revision to Diff 154018. vsapsai added a comment. - Clean up functionality not required by the Standard. https://reviews.llvm.org/D48753 Files: libcxx/include/memory libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp Index: libc

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 154021. vsapsai added a comment. - Proper support for custom allocators without `construct`. https://reviews.llvm.org/D48342 Files: libcxx/include/memory libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp libc

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ Quuxplusone wrote: > vsapsai wrote: > > vsapsai wrote: > > > vsapsai wrote: > > > > erik.pilkington wrote: > > > > > vsapsai wrote: > > > >

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp:60 + + void construct(pointer p, const value_type& val) + { Quuxplusone wrote: > Per my comments on D48342, I think it would b

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added a comment. In https://reviews.llvm.org/D48342#1152063, @EricWF wrote: > Are there any tests which actually exercise the new behavior? Added tests only verify we don't use memcpy erroneously. And existing tests make sure there are no functionality regressions. But there is nothing

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ Quuxplusone wrote: > vsapsai wrote: > > Quuxplusone wrote: > > > vsapsai wrote: > > > > vsapsai wrote: > > > > > vsapsai wrote: > > > > > >

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai updated this revision to Diff 154326. vsapsai added a comment. - Use a better way to detect presence of `construct` with required signature. Clean up tests. Don't know how other compilers will handle this but Clang accepts this C++11-looking-but-accepted-in-C++03 code. https://reviews.

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 154327. vsapsai added a comment. - Clean up tests according to review. We don't need a new test for custom allocators, parent patch covers that. https://reviews.llvm.org/D48342 Files: libcxx/include/memory libcxx/test/std/containers/sequences/vector/v

[PATCH] D47687: [Sema] Missing -Wlogical-op-parentheses warnings in macros (PR18971)

vsapsai added a comment. In https://reviews.llvm.org/D47687#1136351, @Higuoxing wrote: > Sorry, It seems a little bit difficult for me to add a proper fix-it hint for > expressions in macros, because I cannot find the exact position of the last > char of the token on right hand side of operator

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai added inline comments. Comment at: libcxx/include/memory:1470 +decltype(_VSTD::declval<_Alloc>().construct(_VSTD::declval<_Pointer>(), +_VSTD::declval<_Args>())), +void Quuxplusone wrote:

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai updated this revision to Diff 154678. vsapsai added a comment. - Allow allocator `construct` to return a value, not just have return type `void`. https://reviews.llvm.org/D48753 Files: libcxx/include/memory libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_ite

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

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

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai added inline comments. Comment at: libcxx/include/memory:1470 +decltype(_VSTD::declval<_Alloc>().construct(_VSTD::declval<_Pointer>(), +_VSTD::declval<_Args>())), +void Quuxplusone wrote:

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai added a comment. In https://reviews.llvm.org/D48753#1157695, @EricWF wrote: > Why are we doing this? > > I can't find the language in the C++03 specification that requires us to call > an allocators `construct` method if it's present. The main reason I've ended up doing this because I

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai added a comment. In https://reviews.llvm.org/D48753#1157907, @Quuxplusone wrote: > In https://reviews.llvm.org/D48753#1157695, @EricWF wrote: > > > Why are we doing this? > > > > I can't find the language in the C++03 specification that requires us to > > call an allocators `construct` m

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

vsapsai updated this revision to Diff 154912. vsapsai added a comment. - In C++03 call allocator's `destroy` when available. - Rename `_Args` as it's not a variadic template pack. https://reviews.llvm.org/D48753 Files: libcxx/include/memory libcxx/test/std/containers/sequences/vector/vecto

[PATCH] D49119: [Sema][ObjC] Issue a warning when a method declared in a protocol is non-escaping but the corresponding method in the implementation is escaping.

vsapsai added a comment. Overall looks good to me. Maybe add a test when a protocol is declared for an interface, not for a category. Something like __attribute__((objc_root_class)) @interface C4 -(void) m0:(int*) p; // expected-warning {{parameter of overriding method should be annotate

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

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

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

vsapsai added a comment. My review is incomplete, especially I cannot say with confidence if the proposed change is entirely free from unintended consequences that might break code not covered by the test suite. So other reviewers are welcome to chime in. Comment at: include/

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

vsapsai added inline comments. Comment at: include/vector:300 +{ +using _Alloc_traits = allocator_traits<_Alloc>; +for (; __begin1 != __end1; ++__begin1, (void)++__begin2) Quuxplusone wrote: > vsapsai wrote: > > Have you checked why `using` is accepted in

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

vsapsai added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:1875 + // Stop further preprocessing if a fatal error has occurred. Any diagnostics + // we might have raised will not be visible. + if (ShouldEnter && Diags->hasFatalErrorOccurred()) jk

[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.

vsapsai created this revision. vsapsai added reviewers: bruno, benlangmuir. Herald added a subscriber: dexonsmith. Orphaned files prevent us from building a file system tree and cause the assertion > Assertion failed: (NewParentE && "Parent entry must exist"), function > uniqueOverlayTree, file

[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.

vsapsai added inline comments. Comment at: clang/lib/Basic/VirtualFileSystem.cpp:1416 +if (NameValueNode) + error(NameValueNode, "file is not located in any directory"); +return nullptr; Not happy with the error message but didn't come up

[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.

vsapsai added a comment. Need to double check what tests we have when using relative path names at the root level. I'd like to make the behavior consistent because a file name is a specific case of relative paths. So far there are no assertions and no errors but file lookup doesn't seem to be w

[PATCH] D66696: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

vsapsai added a comment. Thanks for the review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66696/new/ https://reviews.llvm.org/D66696 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D66696: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

This revision was automatically updated to reflect the committed changes. Closed by commit rG02c2ab3d8872: [ObjC generics] Fix not inheriting type bounds in categories/extensions. (authored by vsapsai). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D68252: [Stats] Add ALWAYS_ENABLED_STATISTIC enabled regardless of LLVM_ENABLE_STATS.

vsapsai updated this revision to Diff 224235. vsapsai marked an inline comment as done. vsapsai added a comment. Herald added a subscriber: jfb. - Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68252/new/ https://reviews.llvm.org/D68252 Files: clang/include/cl

[PATCH] D68252: [Stats] Add ALWAYS_ENABLED_STATISTIC enabled regardless of LLVM_ENABLE_STATS.

vsapsai marked 2 inline comments as done. vsapsai added a comment. Thanks for the review. Comment at: llvm/include/llvm/ADT/Statistic.h:47 -class Statistic { +class StatisticBase { public: dsanders wrote: > Do we actually need the common base class? I'm thin

[PATCH] D68252: [Stats] Add ALWAYS_ENABLED_STATISTIC enabled regardless of LLVM_ENABLE_STATS.

This revision was automatically updated to reflect the committed changes. vsapsai marked an inline comment as done. Closed by commit rGadb203feda90: [Stats] Add ALWAYS_ENABLED_STATISTIC enabled regardless of LLVM_ENABLE_STATS. (authored by vsapsai). Changed prior to commit: https://reviews.llvm

[PATCH] D69011: Replace platform-dependent `stat` with `llvm::sys::fs::status`. NFC intended.

vsapsai added a comment. The plan is to instrument `llvm::sys::fs::status` with `ALWAYS_ENABLED_STATITSTIC` to be able to catch regressions causing lots of `stat` calls. That's why replacing current `stat` calls. And it seems to be a good change regardless of future plans. CHANGES SINCE LAST

[PATCH] D69011: Replace platform-dependent `stat` with `llvm::sys::fs::status`. NFC intended.

vsapsai created this revision. vsapsai added reviewers: bruno, sammccall. Herald added subscribers: ributzka, arphaman, dexonsmith, jkorous. vsapsai added a comment. The plan is to instrument `llvm::sys::fs::status` with `ALWAYS_ENABLED_STATITSTIC` to be able to catch regressions causing lots of

[PATCH] D69011: Replace platform-dependent `stat` with `llvm::sys::fs::status`. NFC intended.

vsapsai added a comment. Thanks for the review! If `fs::status` behaviour is sufficiently different on Windows, it is worth fixing because I believe majority of non-Windows developers expect them to work in the same way. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69011/new/ https:/

[PATCH] D69011: Replace platform-dependent `stat` with `llvm::sys::fs::status`. NFC intended.

This revision was automatically updated to reflect the committed changes. Closed by commit rG930ada91ce8f: Replace platform-dependent `stat` with `llvm::sys::fs::status`. NFC intended. (authored by vsapsai). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

vsapsai created this revision. vsapsai added reviewers: rsmith, bruno, Bigcheese. Herald added subscribers: ributzka, dexonsmith, jkorous. Herald added a project: clang. HeaderSearch was marking requested HeaderFileInfo as Resolved only based on the presence of ExternalSource. As the result, using

[PATCH] D75323: Support relative dest paths in headermap files

vsapsai added a comment. There is nothing in header maps preventing from using relative paths. For example, I was able to run one of the tests with relative paths like ./bin/clang -cc1 -fsyntax-only ../../../llvm-project/clang/test/Preprocessor/headermap-rel.c -I ./tools/clang/test/Preproces

[PATCH] D80770: [diagtool] Install diagtool when LLVM_INSTALL_TOOLCHAIN_ONLY is ON.

vsapsai created this revision. vsapsai added reviewers: JDevlieghere, steven_wu. Herald added subscribers: ributzka, dexonsmith, jkorous, mgorny. Herald added a project: clang. Not sure about other platforms but `install-xcode-toolchain` was already including diagtool in the toolchain. This change

[PATCH] D80770: [diagtool] Install diagtool when LLVM_INSTALL_TOOLCHAIN_ONLY is ON.

vsapsai added a comment. Thanks for the review, Jonas! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80770/new/ https://reviews.llvm.org/D80770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D80770: [diagtool] Install diagtool when LLVM_INSTALL_TOOLCHAIN_ONLY is ON.

This revision was automatically updated to reflect the committed changes. Closed by commit rG03559c684a9b: [diagtool] Install diagtool when LLVM_INSTALL_TOOLCHAIN_ONLY is ON. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80770/

[PATCH] D82118: [clang][module] Improve incomplete-umbrella warning

vsapsai added a comment. In D82118#2154310 , @zixuw wrote: > In D82118#2154280 , @vsapsai wrote: > > > Didn't get into details but overall GenModuleActionWrapper approach looks > > pretty complicated. Haven't tried

[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

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

[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

vsapsai created this revision. vsapsai added reviewers: rsmith, bruno, Bigcheese. Herald added subscribers: ributzka, dexonsmith, jkorous. Herald added a project: clang. Before the change the diagnostic for module unknown.submodule {} was "error: expected module name" which is incorrect and mi

[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

vsapsai marked 3 inline comments as done. vsapsai added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:700 + "no module named '%0' %select{found|in '%2'}1, " + "expected parent module to be defined before the submodule">; def err_mmap_top_level_in

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

vsapsai added a comment. Thanks for the review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72872/new/ https://reviews.llvm.org/D72872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

This revision was automatically updated to reflect the committed changes. Closed by commit rGa8c8b627f23f: [ObjC generics] Fix not inheriting type bounds in categories/extensions. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7

[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

vsapsai added a comment. Ping. Also suggested clang-format linter changes make the test useless as the order of imports is important. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80263/new/ https://reviews.llvm.org/D80263

[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

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

[PATCH] D82118: [clang][module] Improve incomplete-umbrella warning

vsapsai added a comment. Didn't get into details but overall GenModuleActionWrapper approach looks pretty complicated. Haven't tried to accomplish the same myself, so cannot say if such complexity is warranted or not. What happens with fixits in modules when you don't have GenModuleActionWrappe

[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

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

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

vsapsai added a comment. Seems like this change causes `ninja clean` to fail with the error > ninja: error: remove(include/llvm/Support): Directory not empty Full repro steps are ninja install ninja clean Simpler steps are ninja include/llvm/Support/all ninja clean Repository:

[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

vsapsai updated this revision to Diff 282035. vsapsai added a comment. Rebased the change to make sure it still works. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80263/new/ https://reviews.llvm.org/D80263 Files: clang/lib/Lex/HeaderSearch.cpp

[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

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

[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

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

[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

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

[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

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

[PATCH] D85981: [clang][Modules] Use File Names Instead of inodes As Loaded Module Keys

vsapsai added a comment. Overall, looks like a reasonable approach to solve inode reuse. The problem with filenames is that they might not be canonicalized and the same file can be known by different filenames. I'm trying to think through the consequences in the following scenarios: - same nam

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

vsapsai added a comment. Agree that is a mistake in `NSItemProvider` API. The solution I offer is not to revert the change but to add cc1 flag `-fcompatibility-qualified-id-block-type-checking` and to make the driver for Darwin platforms to add this flag. Thus developers using Apple SDKs will s

[PATCH] D79511: [ObjC] Add compatibility mode for type checking of qualified id block parameters.

vsapsai created this revision. vsapsai added reviewers: arphaman, dexonsmith. Herald added subscribers: ributzka, jkorous. Herald added a project: clang. Commit 73152a2ec20766ac45673a129bf1f5fc97ca9bbe fixed type checking for blocks with qualified id parameters. But there are existing APIs in Appl

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

vsapsai added a comment. Patch for the suggested compatibility flag is available at https://reviews.llvm.org/D79511 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66831/new/ https://reviews.llvm.org/D66831 ___ cfe-com

[PATCH] D79511: [ObjC] Add compatibility mode for type checking of qualified id block parameters.

vsapsai added inline comments. Comment at: clang/test/SemaObjC/block-type-safety.m:170 +genericBlockWithParam = blockWithParam; +blockWithParam = genericBlockWithParam; // expected-error {{incompatible block pointer types assigning to 'void (^)(NSAllArray *)' from 'void

[PATCH] D79511: [ObjC] Add compatibility mode for type checking of qualified id block parameters.

vsapsai updated this revision to Diff 263544. vsapsai added a comment. Make compatibility mode accept correct types per James' Y Knight helpful suggestion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79511/new/ https://reviews.llvm.org/D79511 F

[PATCH] D79511: [ObjC] Add compatibility mode for type checking of qualified id block parameters.

vsapsai updated this revision to Diff 263865. vsapsai added a comment. Squash the commits, so that reviewers can review the entire change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79511/new/ https://reviews.llvm.org/D79511 Files: clang/incl

[PATCH] D79511: [ObjC] Add compatibility mode for type checking of qualified id block parameters.

vsapsai added a comment. Thanks everyone for reviews. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79511/new/ https://reviews.llvm.org/D79511 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D79511: [ObjC] Add compatibility mode for type checking of qualified id block parameters.

This revision was automatically updated to reflect the committed changes. Closed by commit rG6a3469f58d0c: [ObjC] Add compatibility mode for type checking of qualified id block… (authored by vsapsai). Changed prior to commit: https://reviews.llvm.org/D79511?vs=263865&id=264058#toc Repository:

[PATCH] D75323: Support relative dest paths in headermap files

vsapsai added a comment. The issue with this change is that it claims to add functionality that exists already. I.e., the test is passing without the change. The confusing part might be that even if `DirectoryLookup::LookupFile` doesn't find a file for relative destination immediately, it updat

[PATCH] D40677: [libcxx] Make std::basic_istream::get 0-terminate input array in case of error.

vsapsai created this revision. It covers the cases when the sentry object returns false and when an exception was thrown. Corresponding standard paragraph is C++14 [istream.unformatted]p9: [...] In any case, if n is greater than zero it then stores a null character into the next successive lo

[PATCH] D39133: [Sema] Better fix for tags defined inside an enumeration (PR28903).

vsapsai added a comment. Ping. The change can be still applied on trunk without changes, all tests are passing. https://reviews.llvm.org/D39133 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D132550: Changes to code ownership in clang and clang-tidy

vsapsai added inline comments. Comment at: clang/CODE_OWNERS.TXT:128 +H: Bigcheese +D: Modules & Serialization I can also help with Modules & Serialization. Maybe more with deserialization and incorporating the deserialized AST with existing AST. Repository:

[PATCH] D128490: [ODRHash diagnostics] Transform method `ASTReader::diagnoseOdrViolations` into a class `ODRDiagsEmitter`. NFC.

vsapsai added a comment. In D128490#3689958 , @ChuanqiXu wrote: > LGTM then. Thanks for the review! Sorry for not answering earlier, I was on vacation. Now I'll rebase the change and will test it again before committing. Repository: rG LLVM Github M

[PATCH] D128490: [ODRHash diagnostics] Transform method `ASTReader::diagnoseOdrViolations` into a class `ODRDiagsEmitter`. NFC.

vsapsai updated this revision to Diff 457723. vsapsai added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128490/new/ https://reviews.llvm.org/D128490 Files: clang/include/clang/AST/DeclCXX.h clang/include/clang/Serialization

[PATCH] D128490: [ODRHash diagnostics] Transform method `ASTReader::diagnoseOdrViolations` into a class `ODRDiagsEmitter`. NFC.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG246c5a994b75: [ODRHash diagnostics] Transform method `ASTReader::diagnoseOdrViolations` into… (authored by vsapsai). Repository: rG LLVM Github Mo

[PATCH] D128690: [ODRHash diagnostics] Preparation to minimize subsequent diffs. NFC.

vsapsai created this revision. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Specifically, making the following changes: - Turn lambdas calculating ODR hashes into

[PATCH] D128690: [ODRHash diagnostics] Preparation to minimize subsequent diffs. NFC.

vsapsai updated this revision to Diff 440451. vsapsai added a comment. More preparations to minimize subsequent diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128690/new/ https://reviews.llvm.org/D128690 Files: clang/lib/Serialization/ASTRe

[PATCH] D128487: [ODRHash diagnostics] Move repetetive code at lambda calls into lambdas themselves. NFC.

vsapsai added a comment. Thanks for the review! In D128487#3614251 , @ChuanqiXu wrote: > Is it possible to combine the several `DiagNote` into `DiagError`? So that > the code would be further reduced. I am OK to do this kind of change in other > revisi

[PATCH] D128487: [ODRHash diagnostics] Move repetetive code at lambda calls into lambdas themselves. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rG017c068f7899: [ODRHash diagnostics] Move repetetive code at lambda calls into lambdas… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D128487: [ODRHash diagnostics] Move repetetive code at lambda calls into lambdas themselves. NFC.

vsapsai added a comment. In D128487#3617821 , @ChuanqiXu wrote: > No immediate or concert ideas here.. It is hard to do refactoring. I sent > https://reviews.llvm.org/D118437 before to do some simplification for the > dispatch of default template argume

[PATCH] D128488: [ODRHash diagnostics] Split `err_module_odr_violation_mismatch_decl_diff` into per-entity diagnostics. NFC.

vsapsai updated this revision to Diff 441182. vsapsai added a comment. Rebase after blocking change has landed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128488/new/ https://reviews.llvm.org/D128488 Files: clang/include/clang/Basic/Diagnosti

[PATCH] D128489: [ODRHash diagnostics] Move common code for calculating diag locations in `DiagnoseODRMismatch` into a lambda. NFC.

vsapsai added a comment. Thanks for the review! Comment at: clang/lib/Serialization/ASTReader.cpp:10020 +auto GetMismatchedDeclLoc = [](const NamedDecl *Container, + ODRMismatchDecl DiffType, const Decl *D) { + SourceLocation Loc;

[PATCH] D128488: [ODRHash diagnostics] Split `err_module_odr_violation_mismatch_decl_diff` into per-entity diagnostics. NFC.

vsapsai added a comment. Thanks for the review, Chuanqi! Comment at: clang/lib/Serialization/ASTReader.cpp:9642 + // note_module_odr_violation_record + enum ODRCXXRecordDifference { StaticAssertCondition, ChuanqiXu wrote: > Is this specific to C++? That

[PATCH] D128488: [ODRHash diagnostics] Split `err_module_odr_violation_mismatch_decl_diff` into per-entity diagnostics. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rG15cb180dcbf8: [ODRHash diagnostics] Split `err_module_odr_violation_mismatch_decl_diff` into… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D128489: [ODRHash diagnostics] Move common code for calculating diag locations in `DiagnoseODRMismatch` into a lambda. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rG2ceb9c347f14: [ODRHash diagnostics] Move common code for calculating diag locations in… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D129068: [AST] Profiling on constraint expression instead of arguments for TypeConstraint in ASTContext::isSameTemplateParameter

vsapsai added a comment. I don't know enough to say if it is a good approach or not, I'll need to check what we can achieve by modifying `Profile` in `ArgLoc.getArgument().Profile`. Specifically, I'm interested to see if we can use the same `ASTContext` for profile. Also I have a few stylistic

[PATCH] D126266: Mark the file entry invalid, until reread. Invalidate SLocEntry cache, readd it on reread. Do not use translateFile, because it pulls in parts of the pch.

vsapsai added a comment. Hmm, I'm concerned with the pieces added purely for testing. Specifically, `FileEntriesToReread` and TestHelper friend functions. Need to think how else we can test it. Do you intend to support only the error cases like clang-repl> #include "file_with_error.h" // e

[PATCH] D129068: [AST] Profiling on constraint expression instead of arguments for TypeConstraint in ASTContext::isSameTemplateParameter

vsapsai added a comment. After looking at this change more, I was thinking about changing the title from **how** to **what** you are doing. For example, something like "[AST] Accept identical TypeConstraint referring to other template parameters." You can tweak it as you know better what's goin

[PATCH] D129068: [AST] Accept identical TypeConstraint referring to other template parameters.

vsapsai added a comment. Thanks for the changes, they look good! While I was looking how we handle "requires" constraints in other cases, I've noticed that they are suspiciously similar. That's why I offer the following change to unify comparison of the constraint expressions diff --git a/cl

[PATCH] D128690: [ODRHash diagnostics] Preparation to minimize subsequent diffs. NFC.

vsapsai updated this revision to Diff 443403. vsapsai added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128690/new/ https://reviews.llvm.org/D128690 Files: clang/lib/Serialization/ASTReader.cpp Index: clang/lib/Serializatio

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

vsapsai added a comment. In D120874#3404420 , @ChuanqiXu wrote: > @vsapsai @dblaikie I want to make sure if this one would affect clang > modules. Or simply, would the name of clang modules contain `:`? (`:` is the > separator of C++20 modules partition

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

vsapsai added a comment. By the way, tried a module name with a colon and ModuleMapParser doesn't accept it include/module.modulemap:1:12: error: skipping stray token module Test:Colon { ^ include/module.modulemap:1:13: error: expected '{' to start module 'Test' module Test:

[PATCH] D122508: [clang][NFC] Remove unused parameter in `Sema::ActOnDuplicateDefinition`.

vsapsai created this revision. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D122508 Files: clang

[PATCH] D118525: [modules] Merge ObjC interface ivars with anonymous types.

vsapsai added a reviewer: benlangmuir. vsapsai added a subscriber: benlangmuir. vsapsai added a comment. I've run clang with this change on a bunch of code (especially Objective-C code) and there were no regressions. Also adding @benlangmuir to reviewers as people mentioned he was doing some wo

[PATCH] D121176: [ASTStructuralEquivalence] Add support for comparing ObjCCategoryDecl.

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

[PATCH] D118525: [modules] Merge ObjC interface ivars with anonymous types.

vsapsai added a comment. Thanks everyone for the feedback! Especially valuable is opinion on making `ObjCContainerDecl` a lexical decl context. As discussed earlier, not many people can provide feedback on Objective-C-related code and I'm still responsible for any problems caused by this change

[PATCH] D118525: [modules] Merge ObjC interface ivars with anonymous types.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG29444f0444c6: [modules] Merge ObjC interface ivars with anonymous

[PATCH] D130325: [ODRHash] Hash `ObjCMethodDecl` and diagnose discovered mismatches.

vsapsai updated this revision to Diff 447803. vsapsai added a comment. Use `diagnoseSubMismatchMethodParameters` both for Obj-C and C++ methods. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130325/new/ https://reviews.llvm.org/D130325 Files: cl

[PATCH] D128490: [ODRHash diagnostics] Transform method `ASTReader::diagnoseOdrViolations` into a class `ODRDiagsEmitter`. NFC.

vsapsai added a comment. In D128490#3670543 , @ChuanqiXu wrote: > For these indentation changes, maybe it is a good idea to edit the > `.git-blame-ignore-revs` files in the root path. That is a good idea. I'm thinking about a different option: I make my

<    1   2   3   4   5   6   7   8   9   10   >