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

2023-09-19 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D159064: [Modules] Make clang modules for the C standard library headers

2023-09-18 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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

2023-09-18 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D158021: [clang][modules] Mark builtin header 'inttypes.h' for modules

2023-08-18 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-08-10 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

2023-08-09 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D157066: [clang][modules][deps] Create more efficient API for visitation of `ModuleFile` inputs

2023-08-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-08-04 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

2023-08-04 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-08-01 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-07-31 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-07-31 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D154460: [ODRHash] Stop hashing `ObjCMethodDecl::isPropertyAccessor` as it doesn't capture inherent method quality.

2023-07-05 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D154459: [ODRHash] Stop hashing `ObjCMethodDecl::isOverriding` as it doesn't capture inherent method quality.

2023-07-05 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D154459: [ODRHash] Stop hashing `ObjCMethodDecl::isOverriding` as it doesn't capture inherent method quality.

2023-07-05 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D154460: [ODRHash] Stop hashing `ObjCMethodDecl::isPropertyAccessor` as it doesn't capture inherent method quality.

2023-07-04 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-07-04 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D154251: Add a flag to disable "duplicate definition of category" warnings

2023-06-30 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

2023-06-09 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

2023-06-09 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-06-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-06-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-05-30 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

2023-05-25 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-05-25 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

2023-05-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-05-03 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D149709: Fix a typo in head comment of CurPPLexer

2023-05-02 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D149709: Fix a typo in head comment of CurPPLexer

2023-05-02 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

2023-04-26 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

2023-04-19 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-04-19 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-04-19 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D144981: [Driver] Allow to collect `-save-stats` data to a file specified in the environment variable.

2023-03-16 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-03-16 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-03-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D144981: [Driver] Allow to collect `-save-stats` data to a file specified in the environment variable.

2023-03-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-03-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-03-01 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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

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

2023-03-01 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-02-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D144149: [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

2023-02-22 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D144149: [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

2023-02-22 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-02-15 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2023-02-15 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D124286: [modules] Allow parsing a duplicate Obj-C interface if a previous one comes from a hidden [sub]module.

2023-01-20 Thread Volodymyr Sapsai via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGed7a46a8de77: [modules] Allow parsing a duplicate Obj-C interface if a previous one comes… (authored by vsapsai). Repository: rG LLVM Github Monor

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

2023-01-20 Thread Volodymyr Sapsai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6ba4afb4d6f2: [ODRHash] Hash `ObjCInterfaceDecl` and diagnose discovered mismatches. (authored by vsapsai). Changed prior to commit: https://reviews.llvm.org/D140073?vs=487045&id=490868#toc Repository:

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

2023-01-20 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Thanks for the review, Bruno! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124286/new/ https://reviews.llvm.org/D124286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

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

2023-01-19 Thread Volodymyr Sapsai via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. vsapsai marked an inline comment as done. Closed by commit rG160bc160b9b1: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches. (authored by vsapsai). Repos

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

2023-01-19 Thread Volodymyr Sapsai via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf33b5b1bf703: [ODRHash] Detect mismatches in anonymous `RecordDecl`. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

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

2023-01-18 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai marked an inline comment as done. vsapsai added a comment. In D71734#4061228 , @ChuanqiXu wrote: > LGTM generally. It'd better to mention this in the `Potentially Breaking > Changes` section of ReleaseNotes. Thanks for all your efforts during the

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

2023-01-18 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 490354. vsapsai added a comment. `setODRHash(0)` for consistency, mention the change in Potentially Breaking Changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71734/new/ https://reviews.llvm.org/D71734 F

[PATCH] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

2023-01-18 Thread Volodymyr Sapsai via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG304d7307aee1: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple… (authored by vsapsai). Repository: rG LLVM Github Monorepo

[PATCH] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

2023-01-18 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141424/new/ https://reviews.llvm.org/D141424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

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

2023-01-17 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 490036. vsapsai added a comment. Rebase and add assertion in `ODRHash::AddRecordDecl`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71734/new/ https://reviews.llvm.org/D71734 Files: clang/include/clang/AST/

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

2023-01-17 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added inline comments. Comment at: clang/lib/AST/Decl.cpp:4881-4882 + // For RecordDecl the ODRHash is stored in the remaining 26 + // bit of RecordDeclBits, adjust the hash to accomodate. + setODRHash(Hash.CalculateHash() >> 6); + return RecordDeclBits.ODRHash; -

[PATCH] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

2023-01-17 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 490003. vsapsai added a comment. Rebase the patch and trigger pre-commit checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141424/new/ https://reviews.llvm.org/D141424 Files: clang/lib/Sema/SemaType.cpp

[PATCH] D141422: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended.

2023-01-17 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Thanks for the review, Florian! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141422/new/ https://reviews.llvm.org/D141422 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D141422: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended.

2023-01-17 Thread Volodymyr Sapsai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG574a77ae8545: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

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

2023-01-16 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Ping. Also checked the change on a bunch of internal Objective-C/C code using modules - no new errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71734/new/ https://reviews.llvm.org/D71734 _

[PATCH] D141422: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended.

2023-01-16 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141422/new/ https://reviews.llvm.org/D141422 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

2023-01-13 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai accepted this revision. vsapsai added a comment. Confirm that for me on macOS without the fix the test is failing every time, so the test seems to be totally sufficient. Comment at: clang/lib/Parse/ParseDecl.cpp:6997 + D2->getLocation().getRawEncoding(); +

[PATCH] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

2023-01-10 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Want to note that originally the issue was noticed with the modules because we serialize and deserialize `PointerTypeLoc::starLoc` and on deserialization we assert if the value is too big. With uninitialized memory the value can be too big //sometimes// and clang was fa

[PATCH] D141422: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended.

2023-01-10 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Doing this minor clean-up for D141424 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141422/new/ https://reviews.llvm.org/D141422 ___ cfe-commi

[PATCH] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

2023-01-10 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 487959. vsapsai added a comment. clang-format the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141424/new/ https://reviews.llvm.org/D141424 Files: clang/lib/Sema/SemaType.cpp clang/unittests/AST/S

[PATCH] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

2023-01-10 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai created this revision. vsapsai added reviewers: rnk, leonardchan, yonghong-song. 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. Some `TypeLoc`s are considere

[PATCH] D141422: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended.

2023-01-10 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai created this revision. vsapsai added a reviewer: fhahn. Herald added subscribers: StephenFan, tschuett, ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `MatrixTypeLoc` is not "sugar" `T

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

2023-01-06 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140073/new/ https://reviews.llvm.org/D140073 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

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

2023-01-06 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 487045. vsapsai added a comment. Serialize/deserialize `ObjCInterfaceTypeLoc::NameEndLoc` as it is used in diagnostic about a mismatching superclass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140073/new/ h

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

2023-01-06 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 487043. vsapsai added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140073/new/ https://reviews.llvm.org/D140073 Files: clang/include/clang/AST/DeclObjC.h clang/include/clang/AST/ODRDiag

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

2022-12-15 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D140002: [C++20] [Modules] Merging of lambda types in deserialization

2022-12-15 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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

2022-12-15 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D124287: [modules][ODRHash] Compare ODR hashes to detect mismatches in duplicate ObjCInterfaceDecl.

2022-12-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D124286: [modules] Allow parsing a duplicate Obj-C interface if a previous one comes from a hidden [sub]module.

2022-12-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D124289: [modules] Proof of concept in using ODR hash to detect mismatching duplicate ObjCInterfaceDecl during deserialization.

2022-12-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D140073: [ODRHash] Hash `ObjCInterfaceDecl` and diagnose discovered mismatches.

2022-12-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D140055: [ODRHash] Detect mismatches in anonymous `RecordDecl`.

2022-12-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.

2022-12-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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`.

2022-12-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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: [Modules] Handle tag types and complain about bad merges in C/Objective-C mode

2022-12-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D139653: [clang] Set ShowInSystemHeader for module-build and module-import remarks

2022-12-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138970: [clang][Driver] Don't overwrite `DiagnosticsEngine::IgnoreAllWarnings`, rely on `DiagnosticOptions::IgnoreWarnings` value.

2022-12-02 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138970: [clang][Driver] Don't overwrite `DiagnosticsEngine::IgnoreAllWarnings`, rely on `DiagnosticOptions::IgnoreWarnings` value.

2022-12-02 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

2022-11-29 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2022-11-29 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138252: [clang][deps] During scanning don't emit warnings-as-errors that are ignored with diagnostic pragmas.

2022-11-29 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2022-11-29 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

2022-11-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138252: [clang][deps] During scanning don't emit warnings-as-errors that are ignored with diagnostic pragmas.

2022-11-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138252: [clang][deps] During scanning don't emit warnings-as-errors that are ignored with diagnostic pragmas.

2022-11-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

2022-11-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

2022-11-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2022-11-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138252: [clang][deps] During scanning don't emit warnings-as-errors that are ignored with diagnostic pragmas.

2022-11-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138630: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there a no overrides.

2022-11-24 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D138630: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there a no overrides.

2022-11-24 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2022-11-23 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D130327: [ODRHash] Detect duplicate `ObjCProtocolDecl` ODR mismatches during parsing.

2022-11-22 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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] D130327: [ODRHash] Detect duplicate `ObjCProtocolDecl` ODR mismatches during parsing.

2022-11-17 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Thanks for reviews! It was a long journey. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130327/new/ https://reviews.llvm.org/D130327 ___ cfe-commits mailing list cfe-commits@lis

  1   2   3   4   5   6   7   8   9   10   >