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

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

[PATCH] D112915: [clang][modules] Track number of includes per submodule

vsapsai added a comment. As we've discussed earlier, tracking `isImport` shouldn't be done per .pcm and here is the test case https://gist.github.com/vsapsai/a2d2bd19c54c24540495fd9b262106aa I'm not sure it is worth adding the second `#include` as the test fails just with one. Overall, the cha

[PATCH] D113775: [clang][modules] Umbrella with missing submodule: unify implicit & explicit

vsapsai added inline comments. Comment at: clang/test/Modules/missing-submodule.m:3 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs %s -verify +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -F %S/Inputs %s

[PATCH] D114051: Illustrate an alternative for tracking includes per submodule.

vsapsai created this revision. Herald added a subscriber: ributzka. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Aggregated transitive includes are stored in SubmoduleState, includes per submodule for subsequent serialization are

[PATCH] D112915: [clang][modules] Track number of includes per submodule

vsapsai added a comment. Didn't go in-depth for serialization/deserialization. When we add tracking `isImport` on per-submodule basis, do you think AST reading/writing would change significantly? Comment at: clang/include/clang/Lex/ExternalPreprocessorSource.h:45 + /// Retur

[PATCH] D114051: Illustrate an alternative for tracking includes per submodule.

vsapsai updated this revision to Diff 387834. vsapsai added a comment. Handle `BuildingSubmoduleStack` not storing the top-level module. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114051/new/ https://reviews.llvm.org/D114051 Files: clang/incl

[PATCH] D114093: [clang][lex] Refactor check for the first file include

vsapsai added a comment. Have just a few little tweaks, otherwise looks good. And we've discussed that it's possible we don't really need the first time lexing check anymore. But that's a separate issue and I support your decision to preserve the existing behavior and not to increase the scope

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

vsapsai added a comment. I think AST format for `IncludedFiles` was discussed here, so I'll continue here though the bulk of implementation is in D114095 now. Have you compared the size of resulting .pcm files when you are using a bitvector compared to a list

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

vsapsai added a comment. Haven't checked the details of serialization/deserialization. High-level question about different serialization approaches (bitvector vs list of file ids) is in D112915 . Comment at: clang/lib/Lex/Preprocessor.cpp:552

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

vsapsai added inline comments. Comment at: clang/include/clang/Serialization/ASTBitCodes.h:700 + /// Record code for included files. + PP_INCLUDED_FILES = 66, }; Small detail. Do you need to add the new record to `ASTWriter::WriteBlockInfoBlock`? It's not cri

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

vsapsai added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:873 RECORD(PP_TOKEN); + RECORD(PP_INCLUDED_FILES); I believe `PP_INCLUDED_FILES` is located in `AST_BLOCK`. Yes, you write it in `ASTWriter::WritePreprocessor` but after `Stre

[PATCH] D114411: [WIP][modules] Avoid deserializing Decls from hidden (sub)modules.

vsapsai created this revision. Herald added a subscriber: ributzka. vsapsai requested review of this revision. Herald added a project: clang. The test case is for Objective-C but it is likely other languages like C++ are affected as well, just haven't created appropriate test cases. The test case

[PATCH] D114411: [WIP][modules] Avoid deserializing Decls from hidden (sub)modules.

vsapsai updated this revision to Diff 389374. vsapsai added a comment. Fix some failing tests to demonstrate the magnitued of changes better. In some places check if a decl is deserialized successfully as it's not guaranteed. Also tried to make sure a module is marked as visible *before* we try

[PATCH] D114411: [WIP][modules] Avoid deserializing Decls from hidden (sub)modules.

vsapsai updated this revision to Diff 389379. vsapsai added a comment. Mark identifiers as out-of-date not only on loading a new module but on making a (sub)module visible too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114411/new/ https://revi

[PATCH] D114411: [WIP][modules] Avoid deserializing Decls from hidden (sub)modules.

vsapsai updated this revision to Diff 389630. vsapsai added a comment. Switch from checking module visibility to checking if a module is imported. Required to handle not re-exported modules - allow to deserialize decls from them even if the modules aren't visible. Repository: rG LLVM Github M

[PATCH] D114411: [WIP][modules] Avoid deserializing Decls from hidden (sub)modules.

vsapsai abandoned this revision. vsapsai added a comment. After more investigation and comparing different entities (like structs, enums) this approach seems to be too involved and probably wrong. A better choice seems to be following the same direction as in ⦗Modules⦘ Implement ODR-like semant

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

vsapsai added a comment. I've mentioned it in D112915 as we've discussed the stored data format there. But my concern was that bitvector packing might be not the most space-efficient encoding. I haven't done proper testing, just off-the-cuff comparison and it

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

vsapsai created this revision. vsapsai added reviewers: bruno, jansvoboda11, rsmith. Herald added a subscriber: ributzka. vsapsai requested review of this revision. Herald added a project: clang. Fix errors like clang/test/Modules/redefinition-c-tagtypes.m:36:10: error: reference to 'FST' is a

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

vsapsai added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:16216-16223 if (!getLangOpts().CPlusPlus) { // Postpone making the old definition visible until after we // complete parsing the new one and do the struct

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

vsapsai added a comment. After more testing and thinking I've realized we are still not handling anonymous enums properly. On one hand anonymous EnumDecl aren't duplicates and we aren't making a hidden EnumDecl + EnumConstantDecl visible and don't have ambiguity problems. On the other hand, whe

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

vsapsai updated this revision to Diff 391187. vsapsai added a comment. Add a test case for referencing an anonymous enum constant in C++. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114833/new/ https://reviews.llvm.org/D114833 Files: clang/tes

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

vsapsai updated this revision to Diff 391189. vsapsai added a comment. Attempt to restore a previous commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114833/new/ https://reviews.llvm.org/D114833 Files: clang/lib/Sema/SemaDecl.cpp clang/te

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

vsapsai added a comment. As I was trying to replicate C++ behavior I've created a test case ambiguous-anonymous-enum-lookup.cpp. And it results in diagnostics clang/test/Modules/Output/ambiguous-anonymous-enum-lookup.cpp.tmp/test.cpp:6:10: warning: ambiguous use of internal linkage declarati

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

vsapsai added a subscriber: Bigcheese. vsapsai added a comment. Adding Michael who is infinitely better than me in C++. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114833/new/ https://reviews.llvm.org/D114833

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

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

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

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

[PATCH] D104963: [ODR] Fix using uninitialized FunctionTypeBits.FastTypeQuals in FunctionNoProtoType.

vsapsai abandoned this revision. vsapsai added a comment. Herald added a project: All. The issue is addressed in D133586 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104963/new/ https://reviews.llvm.org/D104963

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai updated this revision to Diff 470915. vsapsai added a comment. Rebase and diagnose attribute mismatch for function parameters. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135472/new/ https://reviews.llvm.org/D135472 Files: clang/includ

[PATCH] D134249: [modules] Fix error "malformed or corrupted AST file: 'SourceLocation remap refers to unknown module...'".

vsapsai created this revision. vsapsai added reviewers: bnbarham, 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 a framework can be found at a new lo

[PATCH] D134249: [modules] Fix error "malformed or corrupted AST file: 'SourceLocation remap refers to unknown module...'".

This revision was automatically updated to reflect the committed changes. Closed by commit rGe12f6c26c394: [modules] Fix error "malformed or corrupted AST file: 'SourceLocation remap… (authored by vsapsai). Changed prior to commit: https://reviews.llvm.org/D134249?vs=461435&id=461770#toc Repos

[PATCH] D134249: [modules] Fix error "malformed or corrupted AST file: 'SourceLocation remap refers to unknown module...'".

vsapsai marked an inline comment as done. vsapsai added a comment. Thanks for the review! Comment at: clang/lib/Serialization/ModuleManager.cpp:284-286 for (ModuleIterator victim = First; victim != Last; ++victim) { Modules.erase(victim->File); } Chu

[PATCH] D133586: [clang] initialize type qualifiers for FunctionNoProtoType

vsapsai added a comment. How correct is it to access `isConst`, `isVolatile`, `isRestrict` for `FunctionNoProtoType`? Yes, we can provide some default value but I'm curious if accessing that default value is correct. For the record, I've tried to fix the same problem in https://reviews.llvm.or

[PATCH] D133586: [clang] initialize type qualifiers for FunctionNoProtoType

vsapsai added a comment. Thanks for the explanation, it is good to know the reasons for the current direction. Though it still feels like fixing a symptom and not the root cause. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133586/new/ https://re

[PATCH] D135232: [modules] Allow to validate system headers less often with `-fmodules-validate-once-per-build-session`.

vsapsai created this revision. vsapsai added reviewers: benlangmuir, MaskRay. Herald added subscribers: StephenFan, ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Make flags `-fmodules-validat

[PATCH] D135232: [modules] Allow to validate system headers less often with `-fmodules-validate-once-per-build-session`.

vsapsai added a comment. Here is the documentation for both flags: - https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fmodules-validate-system-headers - https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fmodules-validate-once-per-build-session

[PATCH] D133586: [clang] initialize type qualifiers for FunctionNoProtoType

vsapsai added a comment. In D133586#3833274 , @aaron.ballman wrote: > Personally, I think the next step is to add a local `assert()` to this > function to try to find out why we're calling this on functions without a > prototype and fix up the call sit

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

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

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

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

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

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

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

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

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai created this revision. vsapsai added reviewers: rtrieu, aaron.ballman, ChuanqiXu, 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. Arguments not for

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai added inline comments. Comment at: clang/lib/AST/ODRHash.cpp:479-480 + + llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs), +[](const Attr *A) { return !A->isImplicit(); }); +} I'm not sure `isImplicit` is the best indicator of

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

vsapsai added a comment. In D130324#3843934 , @jansvoboda11 wrote: > Should we also update `ODRHash::isDeclToBeProcessed`? No, as `isDeclToBeProcessed` is only for sub-decls and protocols cannot be nested inside other entities. Looks like nested protoc

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

vsapsai added a comment. Thanks for the review! I'll wait for the blocking D130324 to land before I commit this change. I appreciate your speedy response. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130325/new/

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai added a comment. In D135472#3844688 , @ChuanqiXu wrote: > I guess the reason why we didn't check odr violation for attributes is that > the attributes was not a part of declarations in C++ before. But it is odd to > skip the check for attributes

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:852 +def err_module_odr_violation_attribute : Error< + "%q0 has different definitions in different modules; first difference is " + "%select{definition in module '%2'|defined here}1 found

[PATCH] D135232: [modules] Allow to validate system headers less often with `-fmodules-validate-once-per-build-session`.

This revision was automatically updated to reflect the committed changes. Closed by commit rGa6ebd3083dbf: [modules] Allow to validate system headers less often with `-fmodules-validate… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai updated this revision to Diff 467564. vsapsai added a comment. - Small fixes to address review comments. - Try to make diagnostics more understandable. - Check attributes on typedefs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135472/new/

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai marked 3 inline comments as done. vsapsai added a comment. Given all the discussion about which attributes should be added to ODR hash, I think it is useful at this point to have TableGen infrastructure to get this information from Attr.td. So I'll work on that. Comme

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai updated this revision to Diff 467570. vsapsai added a comment. Fix the starting point of the diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135472/new/ https://reviews.llvm.org/D135472 Files: clang/include/clang/AST/ODRDiagsEmitter.

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai marked an inline comment as done. vsapsai added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:852 +def err_module_odr_violation_attribute : Error< + "%q0 has different definitions in different modules; first difference is " + "%select{defi

[PATCH] D135931: [Attributes] Improve writing `ExprArgument` value.

vsapsai created this revision. vsapsai added reviewers: aaron.ballman, erichkeane, rtrieu. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Instead of dumping `Expr*` memory address, output `Expr` representa

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

vsapsai added a comment. To fix pre-merge errors like error: 'error' diagnostics seen but not expected: File .../clang/test/Modules/Output/odr_hash.cpp.tmp/Inputs/first.h Line 3797: ... found 'AlignedDoublePtr' with attribute ' __attribute__((align_value(0xb7dc9b8)))' posted D135931

[PATCH] D135931: [Attributes] Improve writing `ExprArgument` value.

This revision was automatically updated to reflect the committed changes. Closed by commit rG1783253c41f1: [Attributes] Improve writing `ExprArgument` value. (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135931/new/ https://re

[PATCH] D135931: [Attributes] Improve writing `ExprArgument` value.

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

[PATCH] D136019: [clang][lex] Avoid `DirectoryLookup` copies

vsapsai added a comment. The change looks good to me. And based on the scope of D121685 it should be sufficient. Do you have any ideas for testing? If it would require significant testing infrastructure work, we can do that separately. But if you have some sim

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9c79eab7fdd5: [ODRHash] Hash `ObjCProtocolDecl` and diagnose discovered mismatches. (authored by vsapsai). Changed prior to commit: https://review

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

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

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2662009c87f4: [ODRHash] Hash `ObjCMethodDecl` and diagnose discovered mismatches. (authored by vsapsai). Changed prior to commit: https://reviews.

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

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] D140073: [ODRHash] Hash `ObjCInterfaceDecl` and diagnose discovered mismatches.

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.

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] D141422: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended.

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] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

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] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

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] D141422: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended.

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.

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] D130326: [ODRHash] Hash `ObjCPropertyDecl` and diagnose discovered mismatches.

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

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

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

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdcb71b5e1d13: [ODRHash] Hash `ObjCPropertyDecl` and diagnose discovered mismatches. (authored by vsapsai). Repository: rG LLVM Github Monorepo CH

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

vsapsai added a comment. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130326/new/ https://reviews.llvm.org/D130326 ___ 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.

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. Herald added a subscriber: cfe-commits. Before the fix the scanning would fai

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

vsapsai added a comment. Should admit that approach with `DiagOpts.IgnoreWarnings` is pretty blunt but I'm not aware of any reasons to use something more elaborate, like stripping away all `-Werror=...` flags. So going with a simple change. Repository: rG LLVM Github Monorepo CHANGES SINCE

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa65d5309d5b7: [ODRHash] Detect duplicate `ObjCProtocolDecl` ODR mismatches during parsing. (authored by vsapsai). Repository: rG LLVM Github Monor

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

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

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

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] D141422: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended.

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] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.

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.

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] D141422: [clang][sema][Matrix] Move code from try-cast to `TypeLocVisitor`. NFC intended.

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] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

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] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.

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] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.

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] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

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] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

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] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.

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] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.

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.

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`.

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] D124286: [modules] Allow parsing a duplicate Obj-C interface if a previous one comes from a hidden [sub]module.

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] D140073: [ODRHash] Hash `ObjCInterfaceDecl` and diagnose discovered mismatches.

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.

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] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

vsapsai added a comment. From the perspective of handling `err_export_inline_not_defined` error as a developer what about the following option? export inline void fn_e(); // note: function 'fn_e' exported as 'inline' here // ... module :private; void fn_e() {} // error: definition of

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

vsapsai created this revision. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. It helps to avoid copy-paste mistakes and makes custom code paths more noticeable. Not

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

vsapsai created this revision. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We'll need to add more cases for Objective-C entities and adding everything to `err_mod

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

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

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

vsapsai added a comment. Sorry for changing my mind. I've thought about the errors more and especially about the case mentioned by Chuanqi export module A; [export] inline void func(); I'm afraid it can complicate the implementation but we can achieve some consistency with errors like e

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

vsapsai created this revision. vsapsai added a reviewer: rtrieu. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. No test changes because `err_module_odr_violation_mismatch_decl_unknown` is a catch-all when

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

vsapsai added a comment. We emit `err_module_odr_violation_mismatch_decl_unknown` at https://github.com/llvm/llvm-project/blob/0fbe3f3f486e01448121f7931a4ca29fac1504ab/clang/lib/Serialization/ASTReader.cpp#L11143-L11150 `ODRMismatchDecl` is defined at https://github.com/llvm/llvm-project/blob/0fb

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

vsapsai added a comment. To trigger "function template|different function template" diagnostic we need the entire "case FunctionTemplate" in "switch (FirstDiffType)"

<    3   4   5   6   7   8   9   10   11   >