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

vsapsai updated this revision to Diff 447882. vsapsai added a comment. - Who could have suspected that some lambdas were double-indented. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128490/new/ https://reviews.llvm.org/D128490 Files: clang/inc

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

vsapsai updated this revision to Diff 447892. vsapsai added a comment. - Replicate some creative old indentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128490/new/ https://reviews.llvm.org/D128490 Files: clang/include/clang/AST/DeclCXX.h

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

vsapsai updated this revision to Diff 447899. vsapsai added a comment. - Revert indentation for the second and subsequent lines in lambda-now-method calls. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128490/new/ https://reviews.llvm.org/D128490

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

vsapsai added a comment. OK. Now I've reverted mostly to the old indentation, preserving some of the quirks. Hope it is easier to review now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128490/new/ https://reviews.llvm.org/D128490 _

[PATCH] D128695: [ODRHash diagnostics] Move `ODRDiagsEmitter` to libAST in separate files. NFC.

vsapsai added a comment. In D128695#3675112 , @ChuanqiXu wrote: > I see. It looks like the tricks in `.git-blame-ignore-revs` doesn't work for > this kind of move. I admit I don't look these changes in details. But as long > as it is helpful for future

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

vsapsai planned changes to this revision. vsapsai added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:9577-9584 +static std::string getOwningModuleNameForDiagnostic(const Decl *D) { + // If we know the owning module, use it. + if (Module *M = D->getImported

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

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

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

vsapsai marked 2 inline comments as done. vsapsai added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:9193-9195 - // Otherwise, use the name of the top-level module the decl is within. - if (ModuleFile *M = getOwningModuleFile(D)) -return M->ModuleName;

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. Have 1 punctuation nit (that I'm not sure about), the rest looks good. Comment at: clang/lib/Serialization/ASTReader.cpp:2854 +bool recompileFinalized = +

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG766a08df12c1: [Frontend] Only compile modules if not already finalized (authored by bnbarham, committed by vsapsai). Repository: rG LLVM Github Mo

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

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

[PATCH] D104344: [modules] Track how headers are included by different modules.

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

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

vsapsai added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:818 +// performance. +RecordDecl *Canon = static_cast(RD->getCanonicalDecl()); +if (RD == Canon || Canon->getODRHash() == RD->getODRHash()) During investigation of a

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

vsapsai created this revision. vsapsai added reviewers: rsmith, bruno, teemperor. Herald added a subscriber: ributzka. vsapsai requested review of this revision. Herald added a project: clang. When deserializing a RecordDecl we don't enforce that redeclaration chain contains only a single definiti

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

vsapsai added a comment. Added John McCall in case I'm missing some CodeGen pieces. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106994/new/ https://reviews.llvm.org/D106994 ___ cfe-commits mailing list

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

vsapsai added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:832 + RD->setCompleteDefinition(false); + Reader.mergeDefinitionVisibility(OldDef, RD); +} else { Here is the perfect place to compare if `RD` and `OldDef` are equi

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

vsapsai added a comment. In D106994#2911508 , @rsmith wrote: > For what it's worth, I think the right way to handle this in C is to properly > implement the "compatible types" rule instead of trying to invent something > ODR-ish: in C, struct definition

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

vsapsai added a comment. >> Are there any known signs for mixing lexical and semantic `DeclContext`? I >> plan to test the change on our internal codebase, hopefully it'll help to >> catch any remaining issues. > > The kinds of things I saw go wrong when we were bringing this up on the C++ > si

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

vsapsai planned changes to this revision. vsapsai added a comment. Discovered ambiguous name lookup for IndirectFieldDecl in anonymous structs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106994/new/ https://reviews.llvm.org/D106994

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

vsapsai added a comment. Thanks for the reviews! I'm going to address the comments, `check-lldb`, check tests failing on Debian. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1971 +return false; + unsigned SlotsToCheck = NumArgs > 0 ? NumArgs : 1; + for (unsigne

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

vsapsai added inline comments. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1240 - FieldDecl *Field1, FieldDecl *Field2) { - const auto *Owner2 = cast(Field2->getDeclContext()); By the way, the alternative to intro

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

vsapsai updated this revision to Diff 414244. vsapsai added a comment. Add a comment and an assertion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121176/new/ https://reviews.llvm.org/D121176 Files: clang/lib/AST/ASTStructuralEquivalence.cpp

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

vsapsai marked 2 inline comments as done. vsapsai added a comment. `check-lldb` has the following tests failing lldb-shell :: Expr/TestIRMemoryMap.test lldb-shell :: Process/TestEnvironment.test lldb-shell :: Register/x86-64-gp-read.test lldb-shell :: Register/x86-64-read.test lldb-shel

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

vsapsai added a comment. Thanks for the review, Chuanqi! I believe you were touching recently `isSameEntity`. Do you have any concerns about using `StructuralEquivalenceContext` instead of `isSameEntity`? I've decided to go with `StructuralEquivalenceContext` because at this point we are still

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

vsapsai updated this revision to Diff 414565. vsapsai added a comment. Fix tests on Debian by using `-fobjc-nonfragile-abi` explicitly as declaring ivars in extensions is supported only with non-fragile ABI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

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

vsapsai updated this revision to Diff 414724. vsapsai added a comment. Fix clang-format error. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121176/new/ https://reviews.llvm.org/D121176 Files: clang/include/clang/Testing/CommandLineArgs.h clan

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

vsapsai updated this revision to Diff 414780. vsapsai added a comment. Address comments and make the tests use non-fragile ABI (it's not default everywhere). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121177/new/ https://reviews.llvm.org/D12117

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

vsapsai added a comment. In D121176#3372193 , @martong wrote: > Basically, this looks good to me. But my confidence with ObjC is low, so, I'd > like @shafik as well to approve. > Besides, seems like the pre-merge check fails with the new tests on Debian,

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

vsapsai marked 6 inline comments as done. vsapsai added a comment. In D121177#3371832 , @ChuanqiXu wrote: > I see it is guaranteed by `lookupInstanceVariable(Identifier)` now. I don't > know much about ObjC. But I feel a little bit strange. Is it possibl

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

vsapsai added a comment. Sorry, I'm pretty ignorant in this area but based on > The job construction is altered to build a C++20 header unit (rather than a > PCH file, as would be the case for other headers). I want to clarify. The goal is to allow mixing PCH and PCM files, right? I haven't do

[PATCH] D121465: WIP: [clang][modules] Do not report declarations without linkage as ambiguous

vsapsai added a comment. My concern with this approach is I'm not sure if `isEquivalentInternalLinkageDeclaration` is a desired long-term solution for anonymous C++ enums. If it is, we can piggy-back on it, otherwise hitching our wagon to this train might be undesirable. Also I need to check t

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

vsapsai added a comment. Thanks for explaining the desired interplay between PCH and PCM. The problem I'm seeing is that it can be hard to find correct compiler flags to achieve the desired result (what is enabled and what is disabled). But that is out of scope for this change. =

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

vsapsai accepted this revision. vsapsai added a comment. Thanks for your patience and for all the explanations, I have no other comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121588/new/ https://reviews.llvm.org/D121588 __

[PATCH] D121497: Lex: add support for `{,u}i128` Microsoft extension

vsapsai added a comment. I don't know how "intsafe.h" looks like and how it is involved with the modules (it's not in winsdk.modulemap as far as I can tell). My immediate guess would be that "intsafe.h" doesn't include all the headers that are supposed to provide necessary macros. And if "intsa

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

vsapsai updated this revision to Diff 417468. vsapsai added a comment. Fix bug with anonymous enum constant lookup in C++. Turns out the lookup was broken by using `ObjCInterfaceDecl` as a semantic decl context for anonymous enums. Fixing that fixed the lookup. Also added in tests checking scope

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

vsapsai added a comment. @shafik, can you please check the patch? Or maybe recommend somebody who worked both on ASTStructuralEquivalence and Objective-C part of clang? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121176/new/ https://reviews.llvm

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

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

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

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

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

vsapsai created this revision. vsapsai added reviewers: rsmith, v.g.vassilev, jansvoboda11. Herald added a subscriber: ributzka. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. For redeclaration chains we maintain an invariant of ha

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

vsapsai added a comment. Relevant discussion on how this is used in Swift is in https://github.com/apple/swift/pull/41144 Also it contains links to various [successful] test runs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118855/new/ https://

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

vsapsai added a comment. In D110280#3289431 , @SharonXu wrote: > Hi @vsapsai, I checked out the apple `stable/20211026` llvm-project branch > at https://github.com/apple/llvm-project/tree/stable/20211026, and ran > check-clang locally, but the tested a

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

vsapsai added a comment. `libomptarget :: x86_64-pc-linux-gnu :: mapping/delete_inf_refcount.c` has failed due to > /usr/bin/ld: final link failed: bad value As far as I can tell, it's not related to my change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

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

vsapsai updated this revision to Diff 406977. vsapsai added a comment. Herald added a project: clang. Rebase and address clang-format comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118525/new/ https://reviews.llvm.org/D118525 Files: clan

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

vsapsai added a comment. Ping. Swift PR testing is happy with this change and I've tested it successfully on >100 swift projects. So looks like this change is safe and does what it intends to do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11885

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

vsapsai added a comment. Thanks for the review, Dan! Comment at: clang/include/clang/AST/Decl.h:1383-1398 /// If this definition should pretend to be a declaration. bool isThisDeclarationADemotedDefinition() const { return isa(this) ? false : NonParmVarDeclBit

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

vsapsai updated this revision to Diff 408536. vsapsai added a comment. Update assertion message wording to be more actionable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118855/new/ https://reviews.llvm.org/D118855 Files: clang/include/clang/

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

vsapsai added a comment. In D118855#3320064 , @Bigcheese wrote: > lgtm. Thanks for the review, Michael! I'll give some time to the pre-merge checks to run, then will land the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

This revision was automatically updated to reflect the committed changes. Closed by commit rGfa4a0f1d31e2: [modules] Add a flag for TagDecl if it was a definition demoted to a… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1188

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

vsapsai added a comment. In my testing I was able to find a case where we go around `requires` like // module.modulemap module Main { header "main.h" export * extern module A "extra.modulemap" requires nonsense { extern module B "extra.modulemap" } } // extra

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

vsapsai added a comment. Thanks for the review, Gabor! Now I'll work on rebasing the patch and testing one final time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121176/new/ https://reviews.llvm.org/D121176

[PATCH] D124285: [clang][NFC] In parts of Objective-C Sema use Obj-C-specific types instead of `Decl`.

vsapsai created this revision. vsapsai added reviewers: jansvoboda11, Bigcheese. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D124285 File

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

vsapsai created this revision. vsapsai added reviewers: jansvoboda11, rsmith. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Instead of emitting a redefinition error, check that definitions are equivalent

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

vsapsai created this revision. vsapsai added reviewers: jansvoboda11, rsmith, bruno. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. The purpose of the change is to start using ODR hash for comparison inste

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

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

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

vsapsai added a comment. As a proof of concept how ODR hash comparison can be used during deserialization, please see D124289 . My point is to highlight the amount of reuse we gain from using ODR hash (I particularly enjoy the test case reuse). I agree the err

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

vsapsai added a comment. I also have a very similar change with `ActOnDuplicateDefinition` for `ObjCProtocolDecl`. If anybody is interested in how it compares with `ObjCInterfaceDecl`, I can publish that but it's not finished yet. Comment at: clang/lib/Parse/ParseObjc.cpp:16

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

vsapsai updated this revision to Diff 424561. vsapsai added a comment. Rebase and trigger pre-commit tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121176/new/ https://reviews.llvm.org/D121176 Files: clang/include/clang/Testing/CommandLine

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG4604db9493ff: [ASTStructuralEquivalence] Add support for comparing ObjCCategoryDecl. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121

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

vsapsai updated this revision to Diff 425093. vsapsai added a comment. Rebase and update test to use `ptr` instead of `i64*`. Also now the added test is failing for `implementationIvar`, investigating that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

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

vsapsai added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1261-1263 + Reader.Diag(IVD->getLocation(), diag::err_duplicate_ivar_declaration) + << II; + Reader.Diag(PrevIvar->getLocation(), diag::note_previous_definition); -

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

vsapsai updated this revision to Diff 425303. vsapsai added a comment. Handle in `VisitObjCIvarDecl` only redeclarations in extensions. Interface+interface redeclarations should be handled in VisitObjCInterfaceDecl and implementation+implementation in VisitObjCImplementationDecl. Checked that we

[PATCH] D124285: [clang][NFC] In parts of Objective-C Sema use Obj-C-specific types instead of `Decl`.

vsapsai marked an inline comment as done. vsapsai added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3300 - Decl *ActOnObjCContainerStartDefinition(Decl *IDecl); + void ActOnObjCContainerStartDefinition(ObjCContainerDecl *IDecl); jansvoboda1

[PATCH] D124285: [clang][NFC] In parts of Objective-C Sema use Obj-C-specific types instead of `Decl`.

vsapsai updated this revision to Diff 425334. vsapsai marked an inline comment as done. vsapsai added a comment. Drop unnecessary `cast<>`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124285/new/ https://reviews.llvm.org/D124285 Files: clang/i

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

vsapsai added a comment. In D124287#3474369 , @jansvoboda11 wrote: > This sounds reasonable to me. What use-cases does `ASTStructuralEquivalence` > fit better than ODR hashes? I think the biggest advantage of `ASTStructuralEquivalence` is that it doesn

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd32c685e1012: [modules] Merge equivalent extensions and diagnose ivar redeclarations for… (authored by vsapsai). Repository: rG LLVM Github Monore

[PATCH] D124285: [clang][NFC] In parts of Objective-C Sema use Obj-C-specific types instead of `Decl`.

vsapsai added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17067 +void Sema::ActOnObjCTemporaryExitContainerContext(ObjCContainerDecl *ObjCCtx) { + auto DC = cast(ObjCCtx); assert(DC == CurContext && "Mismatch of container contexts"); jansvoboda11

[PATCH] D124285: [clang][NFC] In parts of Objective-C Sema use Obj-C-specific types instead of `Decl`.

vsapsai updated this revision to Diff 425891. vsapsai added a comment. Simplify pointer comparison. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124285/new/ https://reviews.llvm.org/D124285 Files: clang/include/clang/Parse/Parser.h clang/incl

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

vsapsai added a comment. Recollected that `ASTStructuralEquivalence` by default tends to be order-dependent while ODR hash order-independent. Different approaches can be achieved with different mechanisms but some behavior is easier to achieve. Another peculiarity of `ASTStructuralEquivalence`

[PATCH] D124285: [clang][NFC] In parts of Objective-C Sema use Obj-C-specific types instead of `Decl`.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3b762b3ab8d2: [clang][NFC] In parts of Objective-C Sema use Obj-C-specific types instead of… (authored by vsapsai). Repository: rG LLVM Github Mon

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

vsapsai planned changes to this revision. vsapsai added inline comments. Comment at: clang/lib/AST/ODRHash.cpp:528 + + //FIXME: Hash other interface-specific elements like protocols, etc. + vsapsai wrote: > jansvoboda11 wrote: > > Is this important to implement

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

vsapsai accepted this revision as: vsapsai. vsapsai added a comment. This revision is now accepted and ready to land. My comments are addressed, thanks for making the changes! Please wait for @ilya-biryukov's approval to confirm he has nothing else to add. CHANGES SINCE LAST ACTION https://re

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

vsapsai added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:10621-10626 // Compute the hash of the method as if it has no body. -auto ComputeCXXMethodODRHash = [&Hash](const CXXMethodDecl *D) { - Hash.clear(); - Hash.AddFunc

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

vsapsai updated this revision to Diff 444787. vsapsai added a comment. Rebase and unhoist `ComputeTemplateParameterListODRHash`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128690/new/ https://reviews.llvm.org/D128690 Files: clang/lib/Serializ

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf693874c53c1: [ODRHash diagnostics] Preparation to minimize subsequent diffs. NFC. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHA

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

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

[PATCH] D130138: [modules] Replace `-Wauto-import` with `-Rmodule-include-translation`.

vsapsai created this revision. vsapsai added reviewers: iains, rsmith, benlangmuir, arphaman. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Diagnostic for `-Wauto-import` shouldn't be a warning because it

[PATCH] D130138: [modules] Replace `-Wauto-import` with `-Rmodule-include-translation`.

vsapsai added a comment. Not sure about the name of the flag `-Rmodule-include-translation`, seems to be verbose. So suggestions about the flag name are welcome. On the other hand, "auto-import" is expected to mean something else, for example, adding missing imports

[PATCH] D130138: [modules] Replace `-Wauto-import` with `-Rmodule-include-translation`.

vsapsai added a comment. Looks like GCC has some naming pattern there, thanks for sharing, Iain. So I want to share the pattern I'm aiming for. I'd like the flag to start with `-Rmodule-...` to be consistent with `-Rmodule-build`, `-Rmodule-import`, `-Rmodule-lock`. Repository: rG LLVM Gith

[PATCH] D126731: [pseudo] Eliminate dependencies from clang-pseudo-gen. NFC

vsapsai added a comment. Looks like this breaks a modular build. I.e., cmake -GNinja ~/Projects/llvm/llvm-project/llvm \ -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" \ -DLLVM_ENABLE_ASSERTIONS=ON \ -DCMAKE_BUILD_TYPE=RelWithDebInfo \ -DLLVM_ENABLE_MODULES=ON ninja clangPse

[PATCH] D130138: [modules] Replace `-Wauto-import` with `-Rmodule-include-translation`.

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

[PATCH] D130138: [modules] Replace `-Wauto-import` with `-Rmodule-include-translation`.

This revision was automatically updated to reflect the committed changes. Closed by commit rG381fcaa1365b: [modules] Replace `-Wauto-import` with `-Rmodule-include-translation`. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130

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

vsapsai added a comment. Sorry for the huge amount of changes in ASTReader.cpp but most of them are indentation changes, so I hope it's not too bad. I've tried to minimize the changes unrelated to introducing `ODRDiagsEmitter` but if anybody has other ideas/requests, I'm totally willing to redu

[PATCH] D128695: [ODRHash diagnostics] Move `ODRDiagsEmitter` to libAST in separate files. NFC.

vsapsai added a comment. If anybody knows how to demonstrate the code was moved without modification in process, I'll be happy to do that. Unfortunately, the only thing I've found is that it should be auto-detected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D130324: [ODRHash] Hash `ObjCProtocolDecl` and diagnose discovered mismatches.

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

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

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

[PATCH] D130326: [ODRHash] Hash `ObjCPropertyDecl` and diagnose discovered mismatches.

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

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

vsapsai created this revision. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When during parsing we encountered a duplicate `ObjCProtocolDecl`, we were always emitt

[PATCH] D128695: [ODRHash diagnostics] Move `ODRDiagsEmitter` to libAST in separate files. NFC.

vsapsai added a comment. In D128695#3670552 , @ChuanqiXu wrote: > It looks like we lack a patch to use it in parser, right? Kinda. Things get a little bit more complicated before we can do that. But to see the bigger picture, I've published all planned

[PATCH] D130377: Move "clang/Basic/TokenKinds.h" into a separate top-level module.

vsapsai created this revision. vsapsai added reviewers: sammccall, aprantl, jansvoboda11, iana. 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. Fixes modular build fo

[PATCH] D126731: [pseudo] Eliminate dependencies from clang-pseudo-gen. NFC

vsapsai added a comment. In D126731#3670896 , @sammccall wrote: > Hmm, I also don't know. > The idea here is that we specifically depend only on the TokenKind enum from > TokenKinds.h (which doesn't need any generated headers), not on other headers > fr

[PATCH] D130377: Move "clang/Basic/TokenKinds.h" into a separate top-level module.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1e4478bbea72: Move "clang/Basic/TokenKinds.h" into a separate top-level module. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D130377: Move "clang/Basic/TokenKinds.h" into a separate top-level module.

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

[PATCH] D66696: [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. When a category/extension doesn't repeat a type bound, corresponding type parameter is substituted with `id` when used as a type argument. As a result, in the

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

vsapsai created this revision. vsapsai added reviewers: erik.pilkington, arphaman. Herald added subscribers: ributzka, dexonsmith, jkorous. When checking if block types are compatible, we are checking for compatibility their return types and parameters' types. As these types have different varianc

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

vsapsai marked 2 inline comments as done. vsapsai added inline comments. Comment at: clang/test/SemaObjC/block-type-safety.m:141 genericBlock = block; +block = genericBlock; // expected-error {{incompatible block pointer types assigning to 'NSAllArray *(^)(id)' from 'id

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

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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL370130: [ObjC] Fix type checking for qualified id block parameters. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D66907: [Modules] Fix rebuilding an updated module for each of its consumers.

vsapsai created this revision. vsapsai added reviewers: dexonsmith, bruno, rsmith. Herald added subscribers: ributzka, jkorous. Marking a module for a rebuild when its signature differs from the expected one causes redundant module rebuilds for incremental builds. When a module is updated, its sig

[PATCH] D66907: [Modules] Fix rebuilding an updated module for each of its consumers.

vsapsai added a comment. Earlier `tryToDropPCM` was called `tryToRemoveBuffer` and it was added in r298278

[PATCH] D66907: [Modules] Fix rebuilding an updated module for each of its consumers.

This revision was automatically updated to reflect the committed changes. Closed by commit rL370274: [Modules] Fix rebuilding an updated module for each of its consumers. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

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