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

vsapsai added inline comments. Comment at: clang/test/Driver/save-stats.c:26 // CHECK-LTO: "-plugin-opt=stats-file=save-stats.stats" +// CHECK-LTO: "-plugin-opt=-stats-file-append" ahatanak wrote: > ahatanak wrote: > > Doesn't this require `stats-file-append`

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

vsapsai updated this revision to Diff 503488. vsapsai added a comment. Don't touch LTO pipeline to avoid breaking existing workflows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144981/new/ https://reviews.llvm.org/D144981 Files: clang/docs/Co

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

vsapsai added a comment. Adding folks involved with LTO. The problem is that stats emitted during the first round of compilation are likely to be overwritten by stats emitted during [Thin]LTO itself. Not touching it now to preserve the existing behavior. But wanted to inform you about this shor

[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

vsapsai created this revision. vsapsai added reviewers: martong, shafik, ahatanak. Herald added subscribers: ributzka, rnkovacs. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When this happens, it is i

[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

vsapsai added a comment. Kinda a follow-up to D121176 . One big alternative that I've considered is to store interface name in `ObjCCategoryDecl` because when we parse `@interface InterfaceName(CategoryName)` we know the interface name. The intention was to al

[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

vsapsai added inline comments. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:2062 + *Intf2 = D2->getClassInterface(); + if ((Intf1 != nullptr) != (Intf2 != nullptr)) +return false; shafik wrote: > I think this would be easie

[PATCH] D144149: [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

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. A single class allows multiple catego

[PATCH] D144149: [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

vsapsai added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:4188 + llvm::DenseSet> NonEquivalentDecls; + StructuralEquivalenceContext Ctx( + Cat->getASTContext(), Existing->getASTContext(), Decided to use St

[PATCH] D144149: [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

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

[PATCH] D144149: [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2893d55f8f61: [Serialization] Don't warn when a deserialized category is equivalent to an… (authored by vsapsai). Repository: rG LLVM Github Monor

[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

vsapsai added a comment. In D148776#4328425 , @dblaikie wrote: > Got a link to a design discussion motivating this change? No design discussion. I though that doing less work is not contentious. > I'd have thought it made sense to put modulemaps in subd

[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

vsapsai created this revision. vsapsai added reviewers: EricWF, aprantl, chapuni. Herald added subscribers: ributzka, s.egerton, simoncook, asb, fedor.sergeev, dschuff. Herald added a reviewer: deadalnix. Herald added a project: All. vsapsai requested review of this revision. Herald added subscrib

[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

vsapsai added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:358-373 +#if ALLOW_SUBDIRECTORY_MODULE_MAP_SEARCH // If we've already performed the exhaustive search for module maps in this // search directory, don't do it again. if (Dir.haveSearchedAllMo

[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

vsapsai updated this revision to Diff 515194. vsapsai added a comment. Don't touch HeaderSearch.cpp as the actual change in header search will be a separate patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148776/new/ https://reviews.llvm.org/

[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

vsapsai added a comment. Ping. Want to remind everyone that it is OK to review only a subset of files. The change spans multiple sub-projects and it is understandable nobody is comfortable reviewing //all// of them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D149709: Fix a typo in head comment of CurPPLexer

vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. Thanks for the fix! Don't see any other places in the file to fix. Comment at: clang/include/clang/Lex/Preprocessor.h:726 /// The current top of the stack that we're l

[PATCH] D149709: Fix a typo in head comment of CurPPLexer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe46aa7f92729: Fix a typo in head comment of `CurPPLexer`. (authored by zhouyizhou, committed by vsapsai). Repository: rG LLVM Github Monorepo CHA

[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG24f36a215b4e: [Modules] Move modulemaps to header search directories. NFC intended. (authored by vsapsai). Changed prior to commit: https://review

[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

vsapsai updated this revision to Diff 529735. vsapsai added a comment. Use different check for nullptr + rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151523/new/ https://reviews.llvm.org/D151523 Files: clang/lib/AST/ASTStructuralEquival

[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

vsapsai added a comment. In D151523#4396010 , @ahatanak wrote: > In D151523#4374808 , @vsapsai wrote: > >> Kinda a follow-up to D121176 . >> >> One big alternative that I've consi

[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2e16df352c7a: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have… (authored by vsapsai). Repository: rG LLVM Github Monorepo

[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

vsapsai added a comment. If anybody has further comments about the condition style, I'm happy to change the code post-commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151523/new/ https://reviews.llvm.org/D151523 _

[PATCH] D139653: [clang] Set ShowInSystemHeader for module-build and module-import remarks

vsapsai added a comment. From my experience, `ShowInSystemHeader` for `-Rmodule-build` is overall useful. I haven't heard from people who want no remarks from system headers and the rest is just hypothetical guesses that can go both ways (some people might like one option while some people migh

[PATCH] D71734: [Modules] Handle tag types and complain about bad merges in C/Objective-C mode

vsapsai updated this revision to Diff 482981. vsapsai added a comment. Herald added a project: All. Rebase and use ODRDiagsEmitter. Add more tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71734/new/ https://reviews.llvm.org/D71734 Files: c

[PATCH] D140055: [ODRHash] Detect mismatches in anonymous `RecordDecl`.

vsapsai created this revision. vsapsai added reviewers: bruno, jansvoboda11, ChuanqiXu. 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. Allow completing a redeclarati

[PATCH] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.

vsapsai added a comment. If anybody is curious about anonymous structs/unions, the relevant change is D140055 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71734/new/ https://reviews.llvm.org/D71734 ___

[PATCH] D140055: [ODRHash] Detect mismatches in anonymous `RecordDecl`.

vsapsai added a comment. As I've mentioned `CXXRecordDecl::dataPtr`, the relevant code is https://github.com/llvm/llvm-project/blob/bc63a393262dd278d2f12153c30f8e6ea06a2450/clang/include/clang/AST/DeclCXX.h#L441-L442 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D140073: [ODRHash] Hash `ObjCInterfaceDecl` and diagnose discovered mismatches.

vsapsai created this revision. vsapsai added reviewers: jansvoboda11, ChuanqiXu. 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. When two modules contain interfaces w

[PATCH] D124289: [modules] Proof of concept in using ODR hash to detect mismatching duplicate ObjCInterfaceDecl during deserialization.

vsapsai abandoned this revision. vsapsai added a comment. Superseded by D140073 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124289/new/ https://reviews.llvm.org/D124289

[PATCH] D124286: [modules] Allow parsing a duplicate Obj-C interface if a previous one comes from a hidden [sub]module.

vsapsai updated this revision to Diff 483047. vsapsai added a comment. Implement detecting and diagnosing duplicates during parsing based on ODR hash done during deserialization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124286/new/ https://re

[PATCH] D124287: [modules][ODRHash] Compare ODR hashes to detect mismatches in duplicate ObjCInterfaceDecl.

vsapsai abandoned this revision. vsapsai added a comment. Abandoning this change as now it is split between D124286 and D140073 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D140002: [C++20] [Modules] Merging of lambda types in deserialization

vsapsai added a comment. This is my first and pretty shallow review pass. Need to read the standard more thoroughly to be more useful. Others are welcome to chime in (and as I'll be on vacation are encouraged to chime in). What do you mean by > Due to we don't care about merging unnamed entiti

[PATCH] D140002: [C++20] [Modules] Merging of lambda types in deserialization

vsapsai added a comment. Test case MergeLambdas3.cppm is unstable. Sometimes it passes but usually it fails. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2161-2198 + /// FIXME: The `DeclContext::noload_lookup()` wouldn't load local lexical + /// lookups since unname

[PATCH] D140002: [C++20] [Modules] Merging of lambda types in deserialization

vsapsai added a comment. In D140002#4000233 , @ChuanqiXu wrote: >> Test case MergeLambdas3.cppm is unstable. Sometimes it passes but usually it >> fails. > > Weird. What is the error message if it fails? error: 'error' diagnostics seen but not expec

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

vsapsai updated this revision to Diff 129525. vsapsai added a comment. - Remove curly braces for single-statement ifs. https://reviews.llvm.org/D40677 Files: libcxx/include/istream libcxx/test/std/input.output/iostream.format/input.streams/istream.unformatted/get_pointer_size.pass.cpp l

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

vsapsai marked an inline comment as done. vsapsai added a comment. Thanks for the review. Comment at: libcxx/include/istream:970 } +if (__n > 0) +{ mclow.lists wrote: > I'm not a big fan of "putting braces around single statement blocks

[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.

vsapsai added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2012-2015 +// Skip escaped characters. Escaped newlines will already be processed by +// getAndAdvanceChar. +if (C == '\\') + C = getAndAdvanceChar(CurPtr, Result); rsmith wrote:

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL322326: [libcxx] Make std::basic_istream::get 0-terminate input array in case of error. (authored by vsapsai, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.

This revision was automatically updated to reflect the committed changes. Closed by commit rC322390: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral. (authored by vsapsai, committed by ). Changed prior to commit: https://reviews.llvm.org/D41423?vs=129379&id=129666#toc Repositor

[PATCH] D41311: [CodeGen] Fix crash when a function taking transparent union is redeclared.

vsapsai updated this revision to Diff 130038. vsapsai added a comment. - Try another approach for the fix. It has broader impact but semantically seems to be more correct. Also I think `hasScalarEvaluationKind` should use `Arg->getType()` everywhere as it matches `EmitParmDecl` behaviour. Instea

[PATCH] D41834: [Lex] Fix handling numerical literals ending with ' and signed exponent.

vsapsai added a comment. I've addressed all known issues, please take another look. https://reviews.llvm.org/D41834 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41688: [Lex] Fix crash on code completion in comment in included file.

This revision was automatically updated to reflect the committed changes. Closed by commit rC323008: [Lex] Fix crash on code completion in comment in included file. (authored by vsapsai, committed by ). Changed prior to commit: https://reviews.llvm.org/D41688?vs=128480&id=130710#toc Repository

[PATCH] D41311: [CodeGen] Fix crash when a function taking transparent union is redeclared.

This revision was automatically updated to reflect the committed changes. Closed by commit rC323156: Reland "[CodeGen] Fix crash when a function taking transparent union is… (authored by vsapsai, committed by ). Changed prior to commit: https://reviews.llvm.org/D41311?vs=130038&id=130964#toc R

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

vsapsai added inline comments. Comment at: Parse/ParseTemplate.cpp:492 + // Is there just a typo in the input code? ('typedef' instead of 'typename') + if (Tok.is(tok::kw_typedef)) { +Diag(Tok.getLocation(), diag::err_expected_template_parameter); How does

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

vsapsai added inline comments. Comment at: Parse/ParseTemplate.cpp:492 + // Is there just a typo in the input code? ('typedef' instead of 'typename') + if (Tok.is(tok::kw_typedef)) { +Diag(Tok.getLocation(), diag::err_expected_template_parameter); jkorous-a

[PATCH] D41834: [Lex] Fix handling numerical literals ending with ' and signed exponent.

vsapsai added a comment. Checked that suggested change also fixes another OSS-Fuzz bug https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=4664 https://reviews.llvm.org/D41834 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D42498: [ExprConstant] Fix crash when initialize an indirect field with another field.

vsapsai created this revision. vsapsai added reviewers: rsmith, efriedma. When indirect field is initialized with another field, you have MemberExpr with CXXThisExpr that corresponds to the field's immediate anonymous parent. But 'this' was referring to the non-anonymous parent. So when we were bu

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

vsapsai added inline comments. Comment at: FixIt/fixit-typedef-instead-of-typename-typo.cpp:14 +// CHECK: expected-error@-6 {{unknown type name 'a'}} +// CHECK: expected-error@-6 {{expected member name or ';' after declaration specifiers}} You don't need `// CHE

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. One small note about `-fdiagnostics-parseable-fixits`. The rest looks good to me. And the test looks readable. Comment at: FixIt/fixit-typedef-instead-of-typename-typo.cpp

[PATCH] D42755: [libcxx] Fix last_write_time tests for filesystems that don't support very small times.

vsapsai created this revision. vsapsai added reviewers: EricWF, Hahnfeld. Herald added a subscriber: jkorous-apple. rdar://problem/35865151 https://reviews.llvm.org/D42755 Files: libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp Index: libc

[PATCH] D42755: [libcxx] Fix last_write_time tests for filesystems that don't support very small times.

vsapsai added a comment. Don't like removing `new_time = file_time_type::min() + MicroSec(1);` part of the `test_write_min_time` but it escapes `file_time_type::min()` check. If somebody can explain the purpose of this test, I'd be glad to preserve it for filesystems that support very small tim

[PATCH] D42755: [libcxx] Fix last_write_time tests for filesystems that don't support very small times.

vsapsai added a comment. In https://reviews.llvm.org/D42755#994775, @Hahnfeld wrote: > Can you please add a summary that describes which filesystem this problem can > be seen with? I think outside people can't access rdar tickets, so I can only > guess that it's related to APFS? Further guessin

[PATCH] D41834: [Lex] Fix handling numerical literals ending with ' and signed exponent.

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

[PATCH] D114095: [clang][lex] Include tracking: simplify and move to preprocessor

vsapsai added a comment. In D114095#3261073 , @jansvoboda11 wrote: > @vsapsai do you have any further concerns? My only intended change at this > point is Duncan's suggestion. My understanding was that we were going with ID vectors approach and not wit

[PATCH] D114095: [clang][lex] Include tracking: simplify and move to preprocessor

vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. Looks good to me. Thanks for working on it! Though I haven't built it locally and it can be useful for pre-merge checks to finish. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D112915: [clang][modules] Track included files per submodule

vsapsai added a comment. Can you please rebase this change after D114095 lands? Overall looks good but I want to take one final look and triggering the pre-merge checks will be useful. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D112915: [clang][modules] Track included files per submodule

vsapsai added inline comments. Comment at: clang/include/clang/Lex/Preprocessor.h:1251-1260 + const IncludedFilesSet *getNullSubmoduleIncludes() const { +auto It = IncludedFilesPerSubmodule.find(nullptr); +return It == IncludedFilesPerSubmodule.end() ? nullptr : &It->sec

[PATCH] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

vsapsai added a comment. Haven't really checked the code, at the moment thinking about various failure modes. Cases that aren't tested but I suspect are valid ones: - empty block, i.e., `requires cplusplus {}` - nested blocks. Is it possible to reference external module map from `requires` blo

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

vsapsai created this revision. vsapsai added reviewers: ahatanak, rsmith. vsapsai added a project: clang-modules. Herald added a subscriber: ributzka. vsapsai requested review of this revision. Without the fix ivars with anonymous types can trigger errors like > error: 'TestClass::structIvar' fro

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

vsapsai added a comment. @akyrtzi I believe you were working on the code responsible for decl contexts at some point. I'm not sure you'll be able to review the change. Can you recommend someone else knowledgeable with this code? Repository: rC Clang CHANGES SINCE LAST ACTION https://revie

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

vsapsai added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118525/new/ https://reviews.llvm.org/D118525 ___ 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 inline comments. Comment at: clang/test/Modules/merge-anon-record-definition-in-objc.m:5 +// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks %t/test.m -Wno-objc-property-implementation -Wno-incomplete-implementation \ +// RUN:-fmodules -fmodule-name=Targe

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

vsapsai planned changes to this revision. vsapsai added inline comments. Herald added a project: All. Comment at: clang/test/Modules/merge-anon-record-definition-in-objc.m:5 +// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks %t/test.m -Wno-objc-property-implementation -Wno-incomp

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

vsapsai created this revision. vsapsai added reviewers: martong, shafik, a_sidorin. Herald added subscribers: ributzka, rnkovacs. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Will post shortly another patch that relies on comparing ObjCCate

[PATCH] D121177: [modules] Merge equivalent extensions and diagnose ivar redeclarations for extensions loaded from different modules.

vsapsai created this revision. vsapsai added reviewers: aprantl, ChuanqiXu, rsmith, ahatanak. 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. Emitting metadata for th

[PATCH] D121177: [modules] Merge equivalent extensions and diagnose ivar redeclarations for extensions loaded from different modules.

vsapsai added a comment. The main alternative to the current implementation was to change `getRedeclContext` for ObjCCategoryDecl, so we handle ivar redeclarations and merges in ObjCInterfaceDecl with `ASTDeclReader::findExisting`. Decided against it because of noticeably different merging beha

[PATCH] D114095: [clang][lex] Include tracking: simplify and move to preprocessor

vsapsai added a comment. Thanks for measuring the .pcm impact! And I appreciate including the trunk baseline. The results aren't exactly the same I've seen before, so please forgive me my suspiciousness. When you mention > current trunk with this patch do you mean "just this D114095

[PATCH] D114095: [clang][lex] Include tracking: simplify and move to preprocessor

vsapsai added a comment. In D114095#3188557 , @jansvoboda11 wrote: > Given that, I think we should commit this patch with ID vectors, even though > in isolation (without D112915 ) it's the > worse solution. WDYT? I ha

[PATCH] D114833: [modules] Fix ambiguous name lookup for enum constants from hidden submodules.

vsapsai planned changes to this revision. vsapsai added a comment. In discussions outside of this review the consensus is that "warning: ambiguous use of internal linkage declaration" is correct, so I won't change anything for C++. For Objective-C I still need to find a way to handle anonymous e

[PATCH] D98912: Fix -Winteger-overflow to diagnose regardless of the top-level syntactic form.

vsapsai added a comment. Looks good to me. Have a few clarification questions. Comment at: clang/lib/AST/ExprConstant.cpp:972-973 +// Determine if we might warn that the given expression exhibits undefined +// behavior. +bool mightWarnOnUndefinedBehavior(const Exp

[PATCH] D98912: Fix -Winteger-overflow to diagnose regardless of the top-level syntactic form.

vsapsai accepted this revision. vsapsai added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/ExprConstant.cpp:972-973 +// Determine if we might warn that the given expression exhibits undefined +// behavior. +bool mightWa

[PATCH] D111205: [driver] Explicitly specify `-fbuild-session-timestamp` in seconds.

vsapsai created this revision. vsapsai added a reviewer: benlangmuir. Herald added a subscriber: ributzka. vsapsai requested review of this revision. Herald added a project: clang. Representation of the file's last modification time depends on the file system and isn't guaranteed to be in seconds.

[PATCH] D110280: [modules] Fix IRGen assertion on accessing ObjC ivar inside a method.

vsapsai updated this revision to Diff 377677. vsapsai added a comment. Adhere to clang-format requirements. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110280/new/ https://reviews.llvm.org/D110280 Files: clang/include/clang/AST/DeclObjC.h cl

[PATCH] D110452: [modules] Fix tracking ObjCInterfaceType decl when there are multiple definitions.

vsapsai added a comment. Ping. Failing ORC tests seem to be unrelated, seen the same failed tests in other code reviews. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110452/new/ https://reviews.llvm.org/D110452 __

[PATCH] D110280: [modules] Fix IRGen assertion on accessing ObjC ivar inside a method.

vsapsai added a comment. Are there any comments regarding this change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110280/new/ https://reviews.llvm.org/D110280 ___ cfe-commits mailing list cfe-commits@

[PATCH] D109632: [clang] de-duplicate methods from AST files

vsapsai added a comment. These are interesting results. I'm curious to measure the time spent in `ASTReader::ReadMethodPool`. I'm planning to preserve the order of methods roughly by `s/InstanceMethods.append/InstanceMethods.prepend/` in `ReadMethodPoolVisitor`. `isAcceptableMethodMismatch` is

[PATCH] D110280: [modules] Fix IRGen assertion on accessing ObjC ivar inside a method.

vsapsai added a comment. Thanks for the review! And thanks for the insightful questions about inlined functions. Good to know `static inline` also should trigger the early inline and not only `always_inline`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D110280: [modules] Fix IRGen assertion on accessing ObjC ivar inside a method.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9fad9de5c003: [modules] Fix IRGen assertion on accessing ObjC ivar inside a method. (authored by vsapsai). Repository: rG LLVM Github Monorepo CH

[PATCH] D111476: [modules] Make a module map referenced by a system map a system one too.

vsapsai created this revision. vsapsai added reviewers: Bigcheese, jansvoboda11. Herald added a subscriber: ributzka. vsapsai requested review of this revision. Herald added a project: clang. Mimic the behavior of including headers where a system includer makes an includee a system header too. rd

[PATCH] D111476: [modules] Make a module map referenced by a system map a system one too.

vsapsai added a comment. Used the mechanism simpler than the one with headers on purpose. // The #included file will be considered to be a system header if either it

[PATCH] D110123: [Proof of concept] Serialize fewer transitive methods in `METHOD_POOL`.

vsapsai updated this revision to Diff 379516. vsapsai added a comment. Try to preserve the order of methods when populating the global method pool. This attempt is more successful at keeping the existing code working though I still have 1 unexpected warning that requires further investigation.

[PATCH] D109632: [clang] de-duplicate methods from AST files

vsapsai added a comment. It's good to know `ASTReader::ReadMethodPool` is pretty close for both approaches. And as far as I understand, it includes the code ReadMethodPoolVisitor Visitor(*this, Sel, PriorGeneration); ModuleMgr.visit(Visitor); so the module visiting doesn't seem to be too ex

[PATCH] D111205: [driver] Explicitly specify `-fbuild-session-timestamp` in seconds.

vsapsai added a comment. Ping. Failed tests seem to be unrelated (don't know how this change would cause "flangOmpReport.so: cannot open shared object file: No such file or directory"). If you think it would be better, can rebase and trigger new builds. Repository: rG LLVM Github Monorepo C

[PATCH] D111476: [modules] Make a module map referenced by a system map a system one too.

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

[PATCH] D111860: [modules] Update visibility for merged ObjCProtocolDecl definitions.

vsapsai created this revision. vsapsai added reviewers: Bigcheese, bnbarham. Herald added a subscriber: ributzka. vsapsai requested review of this revision. Herald added a project: clang. Add a test that shows a warning is emitted when we cannot find a visible protocol and merge definition visibil

[PATCH] D111476: [modules] Make a module map referenced by a system map a system one too.

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

[PATCH] D111476: [modules] Make a module map referenced by a system map a system one too.

This revision was automatically updated to reflect the committed changes. Closed by commit rGd0e7bdc20849: [modules] Make a module map referenced by a system map a system one too. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D111205: [driver] Explicitly specify `-fbuild-session-timestamp` in seconds.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG91e19f66e51a: [driver] Explicitly specify `-fbuild-session-timestamp` in seconds. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D111205: [driver] Explicitly specify `-fbuild-session-timestamp` in seconds.

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

[PATCH] D110287: [modules] While merging ObjCInterfaceDecl definitions, merge them as decl contexts too.

vsapsai updated this revision to Diff 380771. vsapsai added a comment. Rebase as the pre-requisite change has landed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110287/new/ https://reviews.llvm.org/D110287 Files: clang/lib/Serialization/ASTRe

[PATCH] D110287: [modules] While merging ObjCInterfaceDecl definitions, merge them as decl contexts too.

vsapsai added a comment. Pre-merge checks are passing now after the rebase. I believe it is a straightforward change and we are merging decl contexts for other Decls already, so merging them for `ObjCInterfaceDecl` makes sense. If there are no objections, I plan to land the change on Friday, Oc

[PATCH] D110287: [modules] While merging ObjCInterfaceDecl definitions, merge them as decl contexts too.

vsapsai added a comment. In D110287#3074082 , @dblaikie wrote: > In D110287#3073804 , @vsapsai wrote: > >> Pre-merge checks are passing now after the rebase. I believe it is a >> straightforward change and we are

[PATCH] D110287: [modules] While merging ObjCInterfaceDecl definitions, merge them as decl contexts too.

vsapsai added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1181-1184 + if (DD.Definition != NewDD.Definition) { +Reader.MergedDeclContexts.insert( +std::make_pair(NewDD.Definition, DD.Definition)); + } rsmith wrote: > A cou

[PATCH] D109632: [clang] de-duplicate methods from AST files

vsapsai added a comment. Sorry for the delay, I haven't finished testing more projects yet but here are my preliminary numbers | **File**| **Baseline (s)** | **Set Dedupe (s)** | **No external (s)** | **Set Dedupe (percentage of baseline)** | **No external (percentage of baseline)** |

[PATCH] D109632: [clang] de-duplicate methods from AST files

vsapsai added a comment. And for the empty module cache the numbers are | **File**| **Baseline (s)** | **Set Dedupe (s)** | **No external (s)** | **Set Dedupe (percentage of baseline)** | **No external (percentage of baseline)** | | Project1. File1 | 14.080673| 13.6296118

[PATCH] D109632: [clang] de-duplicate methods from AST files

vsapsai added a comment. In D109632#3076648 , @rmaz wrote: > In D109632#3076586 , @vsapsai wrote: > >> I'm curious to get the results for an empty module cache because clean >> builds are also important for us. >

[PATCH] D110287: [modules] While merging ObjCInterfaceDecl definitions, merge them as decl contexts too.

vsapsai added a comment. Thanks for the review! I'll commit this and other changes in the stack after testing with the current "main" and making sure the bots on other platforms are happy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110287/new/

[PATCH] D110287: [modules] While merging ObjCInterfaceDecl definitions, merge them as decl contexts too.

vsapsai added a comment. In D110287#3075590 , @dblaikie wrote: > In D110287#3074175 , @vsapsai wrote: > >> What would be a better way to deal with changes in areas where I'm more >> experienced in (Obj-C decl mer

[PATCH] D110287: [modules] While merging ObjCInterfaceDecl definitions, merge them as decl contexts too.

This revision was automatically updated to reflect the committed changes. Closed by commit rGc5931267db26: [modules] While merging ObjCInterfaceDecl definitions, merge them as decl… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D110452: [modules] Fix tracking ObjCInterfaceType decl when there are multiple definitions.

vsapsai updated this revision to Diff 381130. vsapsai added a comment. Rebase and trigger a new round of pre-merge checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110452/new/ https://reviews.llvm.org/D110452 Files: clang/include/clang/AST/

<    5   6   7   8   9   10   11   >