[PATCH] D126566: [ODRHash][NFC] Add missing 'select' case for `ODRMismatchDecl`.

vsapsai added a comment. For the record, `FunctionTemplate` case was added in https://reviews.llvm.org/rG9359e8f22a5403ad9524a92c4ccab4db46c9c100 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126566/new/ https://reviews.llvm.org/D126566 _

[PATCH] D126566: [ODRHash][NFC] Add missing 'select' case for `ODRMismatchDecl`.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf3defc23488e: [ODRHash][NFC] Add missing 'select' case for `ODRMismatchDecl`. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D126566: [ODRHash][NFC] Add missing 'select' case for `ODRMismatchDecl`.

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

vsapsai added a comment. In D159483#4641191 , @iana wrote: > A big assumption this patch makes is that `ModuleMap::isBuiltinHeader` is > primarily to support Apple's unfortunate module needs. Thus this patch turns > that behavior off by default, which m

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

vsapsai added a comment. Still going through the patch and through the discussion. But wanted to ask if there are any complications in reverting this change? For libc++ we've discussed that we don't really know the perf impact of multiple small modules. So I want to make sure we aren't making d

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

vsapsai added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:176 COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax") +LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system modules, and _Builtin_ modules are ignored f

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG97dfaf4cd278: [modules] Fix error about the same module being defined in different .pcm files… (authored by vsapsai). Repository: rG LLVM Github M

[PATCH] D158021: [clang][modules] Mark builtin header 'inttypes.h' for modules

vsapsai added a comment. Running the tests locally fails with lib/clang/18/include/inttypes.h:21:15: fatal error: 'inttypes.h' file not found 21 | #include_next | ^~~~ Is it something wrong with my local configuration? Also want to study the previous discu

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

vsapsai created this revision. vsapsai added reviewers: ChuanqiXu, jansvoboda11. 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. Fix errors like > module 'MultiPath'

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

vsapsai added a comment. Thanks for the quick review! In D156749#4549226 , @ChuanqiXu wrote: > Although there is a FIXME in the definition of `getNameAsRequested()`, it > looks not sense to require you to fix that. It might not be an over burden > for

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

vsapsai added a comment. In D156749#4551596 , @jansvoboda11 wrote: > What are the performance implications of making VFS overlays part of the > context hash? I don't have specific measurements but for Xcode projects different targets have different VF

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

vsapsai added a comment. Ping @jansvoboda11 , @benlangmuir in case you still have thoughts on this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156749/new/ https://reviews.llvm.org/D156749 ___ cfe-com

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

vsapsai added a comment. In D156749#4561803 , @jansvoboda11 wrote: > My suggestion is to use the actual real on-disk path. Not > `FileEntryRef::getName()`, but something that always behaves as if > `use-external-name` was set to `true`. I believe this

[PATCH] D157066: [clang][modules][deps] Create more efficient API for visitation of `ModuleFile` inputs

vsapsai accepted this revision. vsapsai added a comment. The existing `FilenameAsRequested` usage looks good to me. Unfortunately, don't know how to make sure all places where `FilenameAsRequested` is required instead of `Filename` are updated. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

vsapsai added a comment. In D156749#4565994 , @jansvoboda11 wrote: > I think your solution is the most pragmatic. If you're confident this doesn't > break anything internally, I say go for it. But I think it's good to be aware > of the pitfall I mentio

[PATCH] D154251: Add a flag to disable "duplicate definition of category" warnings

vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. As long as there are no other failing tests, looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154251/new/ https://reviews.llvm.

[PATCH] D154459: [ODRHash] Stop hashing `ObjCMethodDecl::isOverriding` as it doesn't capture inherent method quality.

vsapsai created this revision. vsapsai added reviewers: ChuanqiXu, jansvoboda11, Bigcheese. 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. `isOverriding` depends on

[PATCH] D154460: [ODRHash] Stop hashing `ObjCMethodDecl::isPropertyAccessor` as it doesn't capture inherent method quality.

vsapsai created this revision. vsapsai added reviewers: ChuanqiXu, jansvoboda11, Bigcheese. 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. `isPropertyAccessor` depen

[PATCH] D154459: [ODRHash] Stop hashing `ObjCMethodDecl::isOverriding` as it doesn't capture inherent method quality.

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

[PATCH] D154459: [ODRHash] Stop hashing `ObjCMethodDecl::isOverriding` as it doesn't capture inherent method quality.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG18530e5d0770: [ODRHash] Stop hashing `ObjCMethodDecl::isOverriding` as it doesn't capture… (authored by vsapsai). Repository: rG LLVM Github Mono

[PATCH] D154460: [ODRHash] Stop hashing `ObjCMethodDecl::isPropertyAccessor` as it doesn't capture inherent method quality.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf7e0aae7284b: [ODRHash] Stop hashing `ObjCMethodDecl::isPropertyAccessor` as it doesn't… (authored by vsapsai). Repository: rG LLVM Github Monorep

[PATCH] D130327: [ODRHash] Detect duplicate `ObjCProtocolDecl` ODR mismatches during parsing.

vsapsai added a comment. In D130327#3944188 , @NancyWang wrote: > @vsapsai hi Volodymyr Sapsai , test case > clang/test/Modules/hidden-duplicates.m is failing on our llvm community AIX > box > https://lab.llvm.org/buildbot/#/builders/214/builds/4

[PATCH] D138630: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there a no overrides.

vsapsai created this revision. vsapsai added reviewers: egorzhdan, akyrtzi. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Incorrect `isOverriding` flag triggers the assertion `!Overridden.empty()` in `Obj

[PATCH] D138630: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there a no overrides.

vsapsai added a comment. Thanks for the quick review! During the commit I will also fix the typo s/there a no overrides/there are no overrides/ Comment at: clang/test/Modules/override.m:30-34 +@interface CheckOverrideImplementationOfCategory: NSObject +@end +@interface CheckOv

[PATCH] D138630: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there a no overrides.

This revision was automatically updated to reflect the committed changes. Closed by commit rG0314ba3acbab: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there are no… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[PATCH] D138252: [clang][deps] During scanning don't emit warnings-as-errors that are ignored with diagnostic pragmas.

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

[PATCH] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

vsapsai created this revision. vsapsai added reviewers: aaron.ballman, ChuanqiXu, erichkeane. 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. In this change deliberat

[PATCH] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

vsapsai added a comment. This patch builds on top of D135472 and aims to show that hashing attributes with TableGen works fine and doesn't have unexpected problems. As the next step I plan to hash more attributes. Though there is one question regarding argume

[PATCH] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

vsapsai added a comment. In D138859#3954943 , @erichkeane wrote: > I dont have a concern on this in general, but does it cause problems with > modules built? I would think this flag needs to be written in > ASTWriter/picked back up. For an attribute

[PATCH] D138252: [clang][deps] During scanning don't emit warnings-as-errors that are ignored with diagnostic pragmas.

This revision was automatically updated to reflect the committed changes. Closed by commit rGeac90d1236cf: [clang][deps] During scanning don't emit warnings-as-errors that are ignored… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D138252: [clang][deps] During scanning don't emit warnings-as-errors that are ignored with diagnostic pragmas.

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

[PATCH] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

vsapsai added a comment. In D138859#3954975 , @erichkeane wrote: > The hash there isn't the problem, its that you're adding a field to Attr.td > that isn't serialized in ASTWriter/ASTReader. That's a good point. Sorry I haven't realized it at first and

[PATCH] D138970: [clang][Driver] Don't overwrite `DiagnosticsEngine::IgnoreAllWarnings`, rely on `DiagnosticOptions::IgnoreWarnings` value.

vsapsai created this revision. vsapsai added reviewers: bnbarham, dyung, MaskRay. Herald added subscribers: StephenFan, ributzka, arphaman. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Driver overwrit

[PATCH] D138252: [clang][deps] During scanning don't emit warnings-as-errors that are ignored with diagnostic pragmas.

vsapsai added a comment. Ok, I was able to reproduce the error on macOS by adding `-target x86_64-sie-ps5` to one of commands in "clang/test/ClangScanDeps/Inputs/no-werror.json" (wouldn't mind receiving PS5 DevKit, by the way). The problem is that `Driver::BuildCompilation` overwrites `Diagnos

[PATCH] D138970: [clang][Driver] Don't overwrite `DiagnosticsEngine::IgnoreAllWarnings`, rely on `DiagnosticOptions::IgnoreWarnings` value.

vsapsai added a comment. To help folks digging up the history, the code in driver was introduced in https://reviews.llvm.org/D11322 and the test case added in "test/Index/warning-flags.c" requires the change in "clang/tools/libclang/CIndex.cpp", that's why I don't have any extra test changes.

[PATCH] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

vsapsai added a comment. In D138859#3955806 , @ChuanqiXu wrote: > And I think we should add tests for this. e.g., in the ASTImporterTests.cpp. I'm not sure which test would be useful in this case. We kinda already test that different modules agree which

[PATCH] D138970: [clang][Driver] Don't overwrite `DiagnosticsEngine::IgnoreAllWarnings`, rely on `DiagnosticOptions::IgnoreWarnings` value.

This revision was automatically updated to reflect the committed changes. Closed by commit rGb4b54697b7aa: [clang][Driver] Don't overwrite `DiagnosticsEngine::IgnoreAllWarnings`, rely on… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D138970: [clang][Driver] Don't overwrite `DiagnosticsEngine::IgnoreAllWarnings`, rely on `DiagnosticOptions::IgnoreWarnings` value.

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

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

vsapsai added a comment. There should be no error for `blockWithParam = genericBlockWithParam;` because `blockWithParam` is called with `I *` and it is safe to substitute `genericBlockWithParam`. Basically you have I *blockArg; id genericParam = blockArg; And for `genericBlockWithParam = b

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

vsapsai added a comment. Ping. 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://lists.llvm.org/cg

[PATCH] D74939: clang/Modules: Finish renaming CompilerInstance::ModuleManager, NFC.

vsapsai created this revision. vsapsai added reviewers: dexonsmith, bruno, Bigcheese. Herald added subscribers: ributzka, jkorous. Herald added a project: clang. Follow-up to 20d51b2f14ac4488f684f8fc57cb0ba718a6b91d , rename th

[PATCH] D74939: clang/Modules: Finish renaming CompilerInstance::ModuleManager, NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rGa6c8698924d2: clang/Modules: Finish renaming CompilerInstance::ModuleManager, NFC. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74939

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

vsapsai added a comment. In D72872#1872242 , @hans wrote: > [...snip...] > Thanks! I applied the patch and was able to build large parts of Chromium > without any problems. Thanks for checking! Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

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

vsapsai updated this revision to Diff 246005. vsapsai added a comment. - Fix some clang-format checks. Ignore the check to add a space between a generic name and its type parameters as it's not the style the rest of the header uses and not what Apple SDK headers are using. Repository: rG LLV

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

vsapsai added a comment. Ping. 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://lists.llvm.org/cg

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

vsapsai created this revision. vsapsai added reviewers: bruno, Bigcheese. Herald added subscribers: ributzka, dexonsmith, jkorous. Herald added a project: clang. vsapsai marked an inline comment as done. vsapsai added a comment. Looks like other module-related tracking like `LoadedModuleMaps`, `D

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

vsapsai marked an inline comment as done. vsapsai added a comment. Looks like other module-related tracking like `LoadedModuleMaps`, `DirectoryHasModuleMap` works without extra changes. And `ModuleMapParser` relies on a file name to decide if a module map is private or not, so the presence of a

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

This revision was automatically updated to reflect the committed changes. Closed by commit rG4069dd14124e: [modules] Allow frameworks to have only a private module without a public one. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

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

vsapsai added a comment. In D66831#1901972 , @smeenai wrote: > I'm not very familiar with Objective-C semantics. Does saying `@interface J : > I ` mean we're overriding the conformance being specified by `@interface I > `? In that case, the Clang 10 erro

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

vsapsai added a comment. In D66831#1902176 , @smeenai wrote: > Thanks for that explanation; that makes sense. Do you think that the > `@interface J : I` would also ideally be an error then? Theoretically, I think `@interface J : I` should also trigger a

[PATCH] D72860: [modules] Do not cache invalid state for modules that we attempted to load.

vsapsai marked an inline comment as done. vsapsai added inline comments. Comment at: clang/lib/Serialization/ModuleManager.cpp:183 // Get a buffer of the file and close the file descriptor when done. - Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/fal

[PATCH] D69958: Improve VFS compatibility on Windows

vsapsai added a comment. There is one issue with a comment, apart of that everything looks good to me. Looked into replacing slash and backslash literals with calls to `llvm::sys::path::get_separator` or `llvm::sys::path::is_separator` but in my attempts failed to improve existing patch. ===

[PATCH] D72860: [modules] Do not cache invalid state for modules that we attempted to load.

vsapsai marked an inline comment as done. vsapsai added inline comments. Comment at: clang/lib/Serialization/ModuleManager.cpp:183 // Get a buffer of the file and close the file descriptor when done. - Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/fal

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

vsapsai added a comment. What use case are you trying to support? Currently the added test `headermap_relpath.cpp` is passing without the changes to `HeaderSearch.cpp`, so it's not entirely clear what problem it should address. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75323/new/

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

vsapsai added a comment. Ping. 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://lists.llvm.org/cg

[PATCH] D76264: [ObjC generics] Fix missing protocols on type parameter for unparameterized generics.

vsapsai created this revision. vsapsai added reviewers: erik.pilkington, ahatanak. Herald added subscribers: ributzka, jfb, dexonsmith, jkorous. Herald added a project: clang. When a type parameter is used with some protocols, we should keep these protocols both when the generic type is parameteri

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

vsapsai added a comment. In D72872#1868989 , @hans wrote: > I don't have the context here. Was I added as a subscriber because it's > related to the clang 10 release? It's not related to clang 10 release. I've added you because earlier you've found a p

[PATCH] D72860: [modules] Do not cache invalid state for modules that we attempted to load.

vsapsai created this revision. vsapsai added reviewers: bruno, dexonsmith. Herald added subscribers: cfe-commits, ributzka, jkorous. Herald added a project: clang. Partially reverts 0a2be46cfdb698fefcc860a56b47dde0884d5335 as it turned out to cause redundant module rebuilds in multi-process increm

[PATCH] D72860: [modules] Do not cache invalid state for modules that we attempted to load.

vsapsai marked an inline comment as done. vsapsai added a comment. Anecdotal build time measurements before and after the change. First row is a clean build, subsequent rows are incremental builds. | Revision | Before change | After change | Change (after - before) | Relative change | | ---

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

vsapsai created this revision. vsapsai added reviewers: erik.pilkington, ahatanak. Herald added subscribers: ributzka, dexonsmith, jkorous. Herald added a project: clang. When a category/extension doesn't repeat a type bound, corresponding type parameter is substituted with `id` when used as a typ

[PATCH] D72860: [modules] Do not cache invalid state for modules that we attempted to load.

This revision was automatically updated to reflect the committed changes. Closed by commit rG83f4c3af021c: [modules] Do not cache invalid state for modules that we attempted to load. (authored by vsapsai). Changed prior to commit: https://reviews.llvm.org/D72860?vs=238562&id=238668#toc Reposit

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

vsapsai updated this revision to Diff 139233. vsapsai added a comment. Address review comments: - Replace `auto` with explicit type. - Use `llvm::makeArrayRef`. - Remove curly braces around single-statement elses. https://reviews.llvm.org/D44589 Files: clang/include/clang/Basic/SourceLocatio

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

vsapsai marked 3 inline comments as done. vsapsai added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7207-7217 +} else { + FixIts.push_back(FixItHint::CreateInsertion( + SelectorLocs[I], SelectorSlotNames[I])); +

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

vsapsai added a comment. In https://reviews.llvm.org/D44589#1044350, @aaron.ballman wrote: > This generally looks reasonable to me, but @rsmith should weigh in before you > commit because `MultiSourceLocation` is novel. Thanks for the review, Aaron. I tried not to do anything stupid with `Mul

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

vsapsai updated this revision to Diff 139369. vsapsai edited the summary of this revision. vsapsai added a comment. - Implement the same functionality for C++98 and C++03. - Use `DoNotOptimize` instead of `asm`. Didn't move the tests as the standard doesn't require assignment operator to be reent

[PATCH] D43494: [Modules] Fix creating fake definition data for lambdas.

This revision was automatically updated to reflect the committed changes. Closed by commit rC328153: [Modules] Fix creating fake definition data for lambdas. (authored by vsapsai, committed by ). Changed prior to commit: https://reviews.llvm.org/D43494?vs=138059&id=139372#toc Repository: rC

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

vsapsai added inline comments. Comment at: libcxx/include/functional:1722-1733 if (__f.__f_ == 0) __f_ = 0; else if ((void *)__f.__f_ == &__f.__buf_) { __f_ = __as_base(&__buf_); __f.__f_->__clone(__f_); } Here is th

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

vsapsai updated this revision to Diff 139506. vsapsai added a comment. - Replace `function::operator=(nullptr);` with `*this = nullptr;` https://reviews.llvm.org/D34331 Files: libcxx/include/__functional_03 libcxx/include/functional libcxx/test/libcxx/utilities/function.objects/func.wrap

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

vsapsai updated this revision to Diff 139650. vsapsai added a comment. Address review comments: - update comments for tryParseObjCMethodName, use isValidIdentifier. - make MultiSourceLocation more lightweight. I have rebased my patch, so diff between changes can be noisy. https://reviews.llvm.

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

vsapsai marked 2 inline comments as done. vsapsai added inline comments. Comment at: clang/include/clang/Basic/SourceLocation.h:202 +/// Can be used transparently in places where SourceLocation is expected. +class MultiSourceLocation { + bool IsSingleLoc; erik.p

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

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

[PATCH] D44449: [Parser] Fix assertion-on-invalid for unexpected typename.

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

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

vsapsai updated this revision to Diff 139981. vsapsai added a comment. - More tweaks. Remove MultiSourceLocation as nobobody, including me, sees much value in it. https://reviews.llvm.org/D44589 Files: clang/include/clang/Basic/CharInfo.h clang/include/clang/Sema/DelayedDiagnostic.h clan

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

This revision was automatically updated to reflect the committed changes. Closed by commit rL328671: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages. (authored by vsapsai, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

This revision was automatically updated to reflect the committed changes. Closed by commit rC328807: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots. (authored by vsapsai, committed by ). Changed prior to commit: https://reviews.llvm.org/D44589?vs=139981&id=140287#t

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

This revision was automatically updated to reflect the committed changes. Closed by commit rL328807: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots. (authored by vsapsai, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revi

[PATCH] D45013: Generate warning when over-aligned new call is required but not supported.

vsapsai added a comment. Does it make sense to have a warning for `__libcpp_deallocate` and `get_temporary_buffer` too? Not sure about deallocate as you have allocated memory already and allocation has a warning. So for deallocation a warning can be too noisy. But for `get_temporary_buffer` the

[PATCH] D45013: Generate warning when over-aligned new call is required but not supported.

vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D45013#1053998, @EricWF wrote: > In https://reviews.llvm.org/D45013#1052600, @vsapsai wrote: > > > Does it make sense to have a warning for `__libcpp_deallocate` a

[PATCH] D44449: [Parser] Fix assertion-on-invalid for unexpected typename.

vsapsai added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:3076 DS.SetRangeEnd(Tok.getAnnotationEndLoc()); ConsumeAnnotationToken(); // The typename } rsmith wrote: > vsapsai wrote: > > Here we potentially can leave annotati

[PATCH] D44449: [Parser] Fix assertion-on-invalid for unexpected typename.

vsapsai updated this revision to Diff 141448. vsapsai added a comment. - Add a comment. https://reviews.llvm.org/D9 Files: clang/lib/Parse/ParseDecl.cpp clang/test/Parser/cxx-decl.cpp Index: clang/test/Parser/cxx-decl.cpp ===

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

vsapsai added inline comments. Comment at: test/SemaCXX/overloaded-builtin-operators.cpp:95-99 // C++ [over.built]p3 long l1 = lr--; + // C++ [over.built]p4 + float f1 = fr--; Looks like p3 for `lr--` is a typo because p3 is about `++` while p4 is about

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

vsapsai added a comment. Are there more thoughts on this change? https://reviews.llvm.org/D34331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45470: Emit an error when mixing and

vsapsai created this revision. vsapsai added reviewers: rsmith, EricWF, mclow.lists. Herald added subscribers: christof, jkorous-apple. Atomics in C and C++ are incompatible at the moment and mixing the headers can result in confusing error messages. Emit an error explicitly telling about the inc

[PATCH] D45470: Emit an error when mixing and

vsapsai added a comment. In addition to the currently proposed approach #if ... #error ... #endif // header content I also tried #if ... #error ... #else // header content #endif It allows to have fewer error messages but can be more surprising. It can look like the only pro

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. Looks good to me. https://reviews.llvm.org/D44988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D44449: [Parser] Fix assertion-on-invalid for unexpected typename.

This revision was automatically updated to reflect the committed changes. Closed by commit rL329735: [Parser] Fix assertion-on-invalid for unexpected typename. (authored by vsapsai, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D9

[PATCH] D38774: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars.

vsapsai added inline comments. Comment at: clang/lib/CodeGen/CGObjCMac.cpp:5095 +fieldType = fieldType->getAsArrayTypeUnsafe()->getElementType(); + } + rjmccall wrote: > vsapsai wrote: > > rjmccall wrote: > > > You can't just use isa<> here; there can be typ

[PATCH] D38774: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars.

vsapsai updated this revision to Diff 120337. vsapsai added a comment. - Address rjmccall review comment about nested arrays. https://reviews.llvm.org/D38774 Files: clang/lib/CodeGen/CGObjCMac.cpp clang/test/CodeGenObjC/ivar-layout-flexible-array.m Index: clang/test/CodeGenObjC/ivar-layou

[PATCH] D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments

vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. Looks good to me. Repository: rL LLVM https://reviews.llvm.org/D37341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D38774: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars.

This revision was automatically updated to reflect the committed changes. Closed by commit rL316723: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars. (authored by vsapsai). Changed prior to commit: https://reviews.llvm.org/D38774?vs=120337&id=120524#toc Repository: rL LLVM http

[PATCH] D39520: [libcxx][CMake] Fix libc++ build when no LLVM source tree is available.

vsapsai created this revision. Herald added a subscriber: mgorny. Fixes error > CMake Error at test/CMakeLists.txt:52 (configure_lit_site_cfg): > Unknown CMake command "configure_lit_site_cfg". configure_lit_site_cfg is defined in AddLLVM module and not available without LLVM source tree. Rever

[PATCH] D39520: [libcxx][CMake] Fix libc++ build when no LLVM source tree is available.

vsapsai abandoned this revision. vsapsai added a comment. Fixed by changing libcxx build configuration, no CMake change required. https://reviews.llvm.org/D39520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D39776: [libcxx] Mark test cxa_deleted_virtual.pass.cpp as failing for previous libcxx versions.

vsapsai created this revision. r313500 added a fix for undefined "___cxa_deleted_virtual" symbol. Previous libcxx versions don't have the fix and corresponding test should be failing. rdar://problem/34521053 https://reviews.llvm.org/D39776 Files: libcxx/test/libcxx/language.support/cxa_delet

[PATCH] D39776: [libcxx] Mark test cxa_deleted_virtual.pass.cpp as failing for previous libcxx versions.

This revision was automatically updated to reflect the committed changes. Closed by commit rL317734: [libcxx] Mark test cxa_deleted_virtual.pass.cpp as failing for previous libcxx… (authored by vsapsai). Changed prior to commit: https://reviews.llvm.org/D39776?vs=122024&id=122157#toc Repositor

[PATCH] D39279: Stringizing raw string literals containing newline

vsapsai added a comment. On the first glance string erase and insert seem to be done correctly. But we can still improve the tests. Can you please add tests for tricky cases where new line or backslash appear in unexpected places? Some of the examples are R"ab\ cd" R"ab c" R"

[PATCH] D144981: [Driver] Allow to collect `-save-stats` data to a file specified in the environment variable.

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

[PATCH] D144981: [Driver] Allow to collect `-save-stats` data to a file specified in the environment variable.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa845aeb5d6c8: [Driver] Allow to collect `-save-stats` data to a file specified in the… (authored by vsapsai). Repository: rG LLVM Github Monorepo

[PATCH] D144981: [Driver] Allow to collect `-save-stats` data to a file specified in the environment variable.

vsapsai created this revision. vsapsai added reviewers: MatzeB, ahatanak. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Using two environment variables `CC_

[PATCH] D144981: [Driver] Allow to collect `-save-stats` data to a file specified in the environment variable.

vsapsai planned changes to this revision. vsapsai added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:1093 +StatsFile, EC, +llvm::sys::fs::OF_Append | llvm::sys::fs::OF_TextWithCRLF); if (EC) { ahatanak wrote: > `OF_App

[PATCH] D144981: [Driver] Allow to collect `-save-stats` data to a file specified in the environment variable.

vsapsai updated this revision to Diff 501709. vsapsai added a comment. Append to stats file when using `CC_PRINT_INTERNAL_STAT` and for LTO when reusing the same stats file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144981/new/ https://reviews

<    4   5   6   7   8   9   10   11   >