[PATCH] D158948: [clang][ASTImporter] Add import of type-related nodes

danix800 added a comment. @haowei I noticed https://github.com/llvm/llvm-project/pull/65823, is it related to your work? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158948/new/ https://reviews.llvm.org/D158948 __

[PATCH] D158948: [clang][ASTImporter] Add import of type-related nodes

danix800 added a comment. In D158948#4642422 , @haowei wrote: > Looking at > https://github.com/llvm/llvm-project/blob/37a20cc68f545647e614c5ba4ae311dc3fd277e9/clang/lib/Testing/CommandLineArgs.cpp#L47, > this is were the unreachable code was hit. Is it

[PATCH] D159263: [clang-tidy] misc-include-cleaner: avoid duplicated fixes

danix800 added a comment. Duplicated to https://github.com/llvm/llvm-project/pull/65431, abandon this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159263/new/ https://reviews.llvm.org/D159263 ___ c

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 added a comment. Ping~, could anyone have a look at this revision please? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158813/new/ https://reviews.llvm.org/D158813 ___ cfe-commits maili

[PATCH] D159263: [clang-tidy] misc-include-cleaner: avoid duplicated fixes

danix800 updated this revision to Diff 555867. danix800 edited the summary of this revision. danix800 added a comment. 1. Revert to internal set (not using `IncludeCleaner`); 2. Only do deduplication when not `areDiagsSelfContained()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D159263: [clang-tidy] misc-include-cleaner: avoid duplicated fixes

danix800 added a comment. In D159263#4638167 , @kadircet wrote: > Thanks a lot for the patch @danix800 ! > > I initially was rather focused on behavior of this check in workflows that > require seeing "self-contained-diags", but also I rather see the bul

[PATCH] D159263: [clang-tidy] misc-include-cleaner: avoid duplicated fixes

danix800 added a comment. Ping @kadircet~, could you please take a look at this revision? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159263/new/ https://reviews.llvm.org/D159263 ___ cfe-commits mailin

[PATCH] D158499: [analyzer] Compute FAM dynamic size

danix800 added a comment. In D158499#4634166 , @steakhal wrote: > @danix800 FYI I think you used the wrong revision link in the commit. Maybe > mark this revision as "abandoned" again, to reflect the actual status. Sorry for this mistake. D158707

[PATCH] D158499: [analyzer] Compute FAM dynamic size

danix800 added a comment. https://github.com/llvm/llvm-project/commit/12559064e05a11e8418425de59d8745f0cfb1122 mistakingly linked here. The actual revision is D158707 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 added a comment. Thanks for clarifying! If no further comments I'll commit this revison in a day or two! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158707/new/ https://reviews.llvm.org/D158707 _

[PATCH] D159263: [clang-tidy] misc-include-cleaner: fix duplicated fixes

danix800 updated this revision to Diff 555059. danix800 retitled this revision from "[clang-tidy] misc-include-cleaner: remove duplicated includes & fixes" to "[clang-tidy] misc-include-cleaner: fix duplicated fixes". danix800 edited the summary of this revision. danix800 added a comment. 1. Use

[PATCH] D159263: [clang-tidy] misc-include-cleaner: remove duplicated includes & fixes

danix800 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:169 + else +Unused.push_back(&I); + continue; sammccall wrote: > If we want this policy, it should be provided at the include-cleaner library

[PATCH] D159263: [clang-tidy] misc-include-cleaner: remove duplicated includes & fixes

danix800 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:227 + << Inc.SymRef.Target.name(); + if (!AlreadyInserted.contains(Inc.Missing.resolvedPath())) { +DB << FixItHint::CreateInsertion( sammcc

[PATCH] D159263: [clang-tidy] misc-include-cleaner: remove duplicated includes & fixes

danix800 created this revision. danix800 added reviewers: VitaNuo, kadircet, sammccall. danix800 added a project: clang-tools-extra. Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. danix800 requested review of this revisi

[PATCH] D159163: [analyzer][NFC] Workaround miscompilation on recent MSVC

danix800 added a comment. In D159163#4627497 , @steakhal wrote: > Feel free to merge it. Thanks! > I'd be curious to see if this bug is tracked at Microsoft. I searched but no result so I submitted it to MS: https://developercommunity.visualstudio.com/t

[PATCH] D158948: [clang][ASTImporter] Add import of type-related nodes

danix800 updated this revision to Diff 554603. danix800 added a comment. Pulling local matchers for testcases, since https://reviews.llvm.org/D158872 is abandoned. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158948/new/ https://reviews.llvm.org/D158948 Files: clang/lib/AST/ASTImpo

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

danix800 added a comment. In D158872#4627313 , @PiotrZSL wrote: > From a mine perspective when it comes to clang-tidy: > > - I personally do not care about Objective-C specific matchers - I do not > plan to use them. > - I personally avoid using "Type" b

[PATCH] D159163: [analyzer][NFC] Workaround miscompilation on recent MSVC

danix800 created this revision. danix800 added a reviewer: steakhal. danix800 added a project: clang. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald adde

[PATCH] D158948: [clang][ASTImporter] Add import of type-related nodes

danix800 added a comment. Changes planned for https://reviews.llvm.org/D158872 is still under discussion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158948/new/ https://reviews.llvm.org/D158948 ___ cf

[PATCH] D158948: [clang][ASTImporter] Add import of type-related nodes

danix800 updated this revision to Diff 554545. danix800 added a comment. 1. Simplify error handling 2. Remove unnecessary tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158948/new/ https://reviews.llvm.org/D158948 Files: clang/lib/AST/ASTIm

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

danix800 added a comment. In D158872#4626272 , @sammccall wrote: > In D158872#4626095 , @aaron.ballman > wrote: > >> Out of curiosity, are you testing on Windows with MSVC? My understanding on >> build time perf

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 added a comment. In D158707#4621377 , @steakhal wrote: > In D158707#4621270 , @donat.nagy > wrote: > >> In D158707#4621135 , @steakhal >> wrote: >> >>> Oh, so w

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 updated this revision to Diff 554383. danix800 added a comment. 1. Cleanup unnecessary undef/zero checking; 2. Use better defensive API for getting `ConstantArrayType`; 3. Comment on bool argument for readability; 4. Add more test for extent with offset cases. Repository: rG LLVM Gith

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:34 +if (auto SSize = +SVB.convertToArrayIndex(*Size).getAs()) + return *SSize; donat.nagy wrote: > I think it's a good convention if `getDynamicExtent

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

danix800 added a comment. In D158872#4622124 , @aaron.ballman wrote: > Are these matchers going to be used in-tree (by clang-tidy, or something > else)? We typically do not add new AST matches until there's a need for them > because the AST matchers ha

[PATCH] D158953: [analyzer] MmapWriteExecChecker: use getAs instead of castAs

danix800 updated this revision to Diff 553798. danix800 added a comment. Simplify testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158953/new/ https://reviews.llvm.org/D158953 Files: clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.c

[PATCH] D158953: [analyzer] MmapWriteExecChecker: use getAs instead of castAs

danix800 added a comment. In D158953#4620125 , @steakhal wrote: > Could you please simplify the test case? > You could basically get rid of everything there, except for forwarding one > top level parameter as the proto argument to the mmap call. No prob

[PATCH] D158953: [analyzer] MmapWriteExecChecker: use getAs instead of castAs

danix800 created this revision. danix800 added a reviewer: steakhal. danix800 added a project: clang. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald adde

[PATCH] D158948: [clang][ASTImporter] Add import of type-related nodes

danix800 created this revision. danix800 added a reviewer: balazske. danix800 added a project: clang. Herald added a subscriber: martong. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: All. danix800 requested review of this revision. Herald added a subs

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

danix800 updated this revision to Diff 553779. danix800 added a comment. Update doc: fix improper naming in objc testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158872/new/ https://reviews.llvm.org/D158872 Files: clang/docs/LibASTMatchers

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

danix800 updated this revision to Diff 553697. danix800 added a comment. Update docs for `bitIntType` & `dependentBitIntType`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158872/new/ https://reviews.llvm.org/D158872 Files: clang/docs/LibASTMat

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

danix800 added a comment. In D158872#4618252 , @PiotrZSL wrote: > LGTM. > Nothing fancy, looks straight forward. > You may want to wait for an Aaron Ballman opinion. Thanks for the reviewing. Ping @aaron.ballman, could you please take a look at this rev

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

danix800 created this revision. Herald added a subscriber: ChuanqiXu. Herald added a project: All. danix800 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add a few type-related Matchers - bitIntType - constantMatrixType - dependentAddres

[PATCH] D158858: [analyzer] MPIChecker: add defensive checking (check against nullptr)

danix800 added a comment. In D158858#4617510 , @steakhal wrote: > Makes sense. > Would thus test crash without the early returns? And now they wouldn't? Yes, the original crash shown in https://godbolt.org/z/39P7W6KPa Repository: rG LLVM Github Monor

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:195 + const NonLoc Idx = Ctx.getSValBuilder().makeArrayIndex(i); + auto CountReached = SVB.evalBinOp(State, BO_GE, Idx, Count, ASTCtx.BoolTy) +

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:192-198 +SVal Count = CE.getArgSVal(0); +for (size_t i = 0; i < ArrSize; ++i) { + const NonLoc Idx = Ctx.getSValBuilder().makeArrayIndex(i); + auto CountReac

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 added inline comments. Comment at: clang/test/Analysis/mpichecker.cpp:286-288 + for (int i = 0; i < 3; ++i) +MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, +&rs.req[i]); steakhal wrote: > This loop here im

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:189-190 +ASTCtx.getCharWidth(); +const NonLoc MROffset = +SVB.makeArrayIndex(MR->getAsOffset().getOffset() / ElemSizeInBits); steakhal w

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:185-188 +CharUnits ElemSizeInChars = ASTCtx.getTypeSizeInChars(ElemType); +int64_t ElemSizeInBits = +(ElemSizeInChars.isZero() ? 1 : ElemSizeInChars.getQuantit

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:46-48 +if (!ErrorNode) + return; + steakhal wrote: > If these hunks are not closely related to the issue you intend to fix in this > PR, I'd suggess

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 added a comment. In D158813#4616816 , @steakhal wrote: > Let me try to summarize some of the variables: > > So, given an invocation like `MPI_Waitall(C, Requests, Statues)`: > > - `MR` is `lval(Requests)` > - `ElementCount` is the number of eleme

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 updated this revision to Diff 553514. danix800 edited the summary of this revision. danix800 added a comment. 1. Move out complicated computation into separate function; 2. Only check `ConreteInt` request count. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D158858: [analyzer] MPIChecker: add defensive checking (check against nullptr)

danix800 created this revision. danix800 added reviewers: donat.nagy, steakhal. danix800 added a project: clang. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald added

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

danix800 created this revision. danix800 added reviewers: steakhal, donat.nagy, dcoughlin. danix800 added a project: clang. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. H

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 updated this revision to Diff 553351. danix800 edited the summary of this revision. danix800 added a comment. `MPIChecker` is not strictly related to this revision. Will be moved into another revison. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 updated this revision to Diff 553335. danix800 edited the summary of this revision. danix800 added a comment. 1. `getDynamicExtent()` can return both signed/unsigned results. They are converted to signed version as `ArrayIndexType` to keep consistency. All other APIs return this version

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 added a comment. In D158707#4614100 , @steakhal wrote: > As a general comment on requiring all extents to be of unsigned APSInts. > Checkers, like the `MallocChecker`, binds the result of arbitrary > expression's values (the argument of malloc,

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:92 + + return ElementCount.castAs(); +} donat.nagy wrote: > Are you sure that this cannot cause crashes? (E.g. did you check that there > is no corner case when `getElem

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 added inline comments. Comment at: clang/test/Analysis/array-bound-v2-constraint-check.c:1 -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection \ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,alpha.security.Array

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:180-183 + SVal CountReached = + SVB.evalBinOp(State, BO_GE, Idx, Count, ASTCtx.BoolTy); + if (!CountReached.isUndef() && + State->assume(*CountReac

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:202-203 // CHECK UPPER BOUND - DefinedOrUnknownSVal Size = getDynamicExtent(state, Reg, svalBuilder); + SVal Size = svalBuilder.convertToArrayIndex( + getDynamicExtent(

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 added a comment. In D158707#4613743 , @donat.nagy wrote: > Thanks for creating this commit, it's a useful improvement! > > I added some inline comments on minor implementation details; moreover, note > that "Releted checkers:" (instead of "rela

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

danix800 created this revision. danix800 added a project: clang. Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. danix800

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

danix800 added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:4105 - T TypeOrDecl = GetCanTypeOrDecl(FD); - - for (const FriendDecl *FoundFriend : RD->friends()) { + for (FriendDecl *FoundFriend : RD->friends()) { if (FoundFriend == FD) { ba

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

danix800 updated this revision to Diff 552678. danix800 added a comment. Add `const` qualifier & replace `auto` with explicit type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157684/new/ https://reviews.llvm.org/D157684 Files: clang/lib/AST/A

[PATCH] D158499: [analyzer] Compute FAM dynamic size

danix800 added a comment. > I suspected that the wrong cast modeling and how we infer what value ranges > are calculated is susceptible to such APSInt signedness issues, but I haven't > seen a case in the wild for extents. Thus, I didn't think of fixing it > either. But yes, we should. I think

[PATCH] D158499: [analyzer] Compute FAM dynamic size

danix800 added a comment. One of the observable issue with inconsistent size type is void clang_analyzer_eval(int); typedef unsigned long long size_t; void *malloc(unsigned long size); void free(void *); void symbolic_longlong_and_int0(long long len) { char *a = malloc(5);

[PATCH] D158499: [analyzer] Compute FAM dynamic size

danix800 added a comment. We have `getDynamicExtentWithOffset` to use, which can handle more general dynamic memory based cases than this fix. I'll abandon this one. There are issues worth clarifying and fixing: 1). Debugging APIs like `clang_analyzer_dumpExtent` in `ExprInspection` might be e

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

danix800 updated this revision to Diff 552372. danix800 added a comment. 1. Rebase; 2. Replace `auto` with explicit type; 3. Use local var without interrupting `Importer` state. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157684/new/ https://revi

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

danix800 added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:4114 SmallVector ImportedEquivalentFriends; - - while (ImportedFriend) { -bool Match = false; -if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) { - Match = - IsStructural

[PATCH] D158499: [analyzer] Compute FAM dynamic size

danix800 added a comment. In D158499#4606291 , @steakhal wrote: > Thanks for submitting this. > A funny thing is that in my free time I was also working on this last week. > I'll have a look at this more in depth during the week. > For the mean time here

[PATCH] D158499: [analyzer] Compute FAM dynamic size

danix800 updated this revision to Diff 552288. danix800 added a comment. Update testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158499/new/ https://reviews.llvm.org/D158499 Files: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp clang/te

[PATCH] D158499: [analyzer] Compute FAM dynamic size

danix800 created this revision. danix800 added a project: clang. Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. danix800

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

danix800 added a comment. ping~ @balazske @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157684/new/ https://reviews.llvm.org/D157684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D158302: [clang] Add simple utils to ensure static assert on bit count of DeclContext

danix800 updated this revision to Diff 551738. danix800 added a comment. Apply git-clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158302/new/ https://reviews.llvm.org/D158302 Files: clang/include/clang/AST/DeclBase.h clang/include/

[PATCH] D158302: [clang] Add simple utils to ensure static assert on bit count of DeclContext

danix800 updated this revision to Diff 551737. danix800 added a comment. Remove ununsed code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158302/new/ https://reviews.llvm.org/D158302 Files: clang/include/clang/AST/DeclBase.h clang/include/cl

[PATCH] D158302: [clang] Add simple utils to ensure static assert on bit count of DeclContext

danix800 created this revision. danix800 added a reviewer: aaron.ballman. danix800 added a project: clang. Herald added a subscriber: kristof.beyls. Herald added a project: All. danix800 requested review of this revision. Herald added a subscriber: cfe-commits. Manually counting bits for DeclConte

[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

danix800 updated this revision to Diff 551078. danix800 added a comment. Remove debug code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158145/new/ https://reviews.llvm.org/D158145 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/D

[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

danix800 added inline comments. Comment at: clang/unittests/AST/DeclTest.cpp:368 + ASTContext &Ctx = AST->getASTContext(); + Ctx.getTranslationUnitDecl()->dump(); + balazske wrote: > This dump is not needed? Yeah it's for debug only, I'll remove it. Repositor

[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

danix800 updated this revision to Diff 551037. danix800 added a comment. 1. Update ReleaseNotes.rst 2. Fix typos pointed out by @cor3ntin (thanks!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158145/new/ https://reviews.llvm.org/D158145 Files:

[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

danix800 added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:7836 +TEST_P(ASTImporterOptionSpecificTestBase, +ImportFunctionDeclBitShouldNotStampingOnCtorDeclBits) { + Decl *From, *To; cor3ntin wrote: > danix800 wrote: > > cor3ntin wro

[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

danix800 added a comment. I also investigated whether we could count those bits at compile time and statically assert on them, because a small typo or missed update could spend us a lot of time to dig for the cause. My first step is trying to count number of bits for a single bitfield, this is

[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

danix800 added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:7836 +TEST_P(ASTImporterOptionSpecificTestBase, +ImportFunctionDeclBitShouldNotStampingOnCtorDeclBits) { + Decl *From, *To; cor3ntin wrote: > `ImportFunctionDeclBitShouldNotO

[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

danix800 added a comment. > Can you an an entry in `clang/docs/ReleaseNotes.rst` (mentioning the github > issue) No problem! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158145/new/ https://reviews.llvm.org/D158145 _

[PATCH] D156461: [clang][ASTImporter] Merge implicit ctors with definition

danix800 added a comment. In D156461#4583806 , @balazske wrote: > Normally what should happen is that a new move constructor is imported (with > a definition) and linked after the existing one (and the existing is not > modified). Does this violate the

[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

danix800 updated this revision to Diff 551016. danix800 added a comment. Add alternative testcase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158145/new/ https://reviews.llvm.org/D158145 Files: clang/include/clang/AST/DeclBase.h clang/lib/S

[PATCH] D156461: [clang][ASTImporter] Merge implicit ctors with definition

danix800 added a comment. In D156461#4583806 , @balazske wrote: > It looks not good to remove an invalid node from the DeclContext that > otherwise remains in the AST. I checked the problem and found that the > existing move constructor (originally in t

[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

danix800 created this revision. danix800 added reviewers: aaron.ballman, cor3ntin, ychen, balazske. danix800 added a project: clang. Herald added a reviewer: shafik. Herald added a project: All. danix800 requested review of this revision. Herald added a subscriber: cfe-commits. `FunctionDeclBitfie

[PATCH] D157691: [ASTImporter] Remove extranous FunctionTemplateDecl introduced by templated friend

danix800 updated this revision to Diff 550210. danix800 added a comment. 1. Remove unnecessary testcase. 2. Format unittest code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157691/new/ https://reviews.llvm.org/D157691 Files: clang/lib/AST/AST

[PATCH] D157691: [ASTImporter] Remove extranous FunctionTemplateDecl introduced by templated friend

danix800 added a comment. In D157691#4587490 , @balazske wrote: > When committing this patch, the commit message should not contain the whole > AST and crash dump (phabricator takes the "summary" text), this looks too > much in a commit message. One AST

[PATCH] D157777: [ASTMatcher] Add matcher for 'MacroQualifiedType'

danix800 updated this revision to Diff 550178. danix800 added a comment. 1. Improve matcher description 2. Add a counter-example for test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15/new/ https://reviews.llvm.org/D15 Files: clang/doc

[PATCH] D157777: [ASTMatcher] Add matcher for 'MacroQualifiedType'

danix800 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7261 +/// Matches macro qualified types. +/// aaron.ballman wrote: > How about: Matches qualified types when the qualifier is applied via a macro. > > and then a second e

[PATCH] D157691: [ASTImporter] Remove extranous FunctionTemplateDecl introduced by templated friend

danix800 updated this revision to Diff 550160. danix800 added a comment. 1. Add unit testcase 2. Use better API Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157691/new/ https://reviews.llvm.org/D157691 Files: clang/lib/AST/ASTImporter.cpp cla

[PATCH] D157780: [ASTImporter] Add import of MacroQualifiedType

danix800 updated this revision to Diff 549609. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157780/new/ https://reviews.llvm.org/D157780 Files: clang/lib/AST/ASTImporter.cpp clang/unittests/AST/ASTImporterTest.cpp Index: clang/unittests/AST/A

[PATCH] D157780: [ASTImporter] Add import of MacroQualifiedType

danix800 created this revision. danix800 added a project: clang. Herald added a subscriber: martong. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: All. danix800 requested review of this revision. Herald added a subscriber: cfe-commits. Add import of M

[PATCH] D157777: [ASTMatcher] Add matcher for 'MacroQualifiedType'

danix800 updated this revision to Diff 549607. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15/new/ https://reviews.llvm.org/D15 Files: clang/docs/LibASTMatchersReference.html clang/docs/ReleaseNotes.rst clang/include/clang/ASTMatcher

[PATCH] D157777: [ASTMatcher] Add matcher for 'MacroQualifiedType'

danix800 updated this revision to Diff 549605. danix800 added a comment. Herald added a subscriber: martong. Herald added a reviewer: shafik. Add import of MacroQualifiedType. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15/new/ https://review

[PATCH] D157777: [ASTMatcher] Add matcher for 'MacroQualifiedType'

danix800 created this revision. danix800 added a project: clang. Herald added a project: All. danix800 requested review of this revision. Herald added a subscriber: cfe-commits. Add matcher for 'MacroQualifiedType' Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D15 Files:

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

danix800 updated this revision to Diff 549436. danix800 added a comment. Turn off `Complain` mode on `IsEquivalentFriend` checking. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157684/new/ https://reviews.llvm.org/D157684 Files: clang/lib/AST/A

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

danix800 updated this revision to Diff 549376. danix800 added a comment. Apply git-clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157684/new/ https://reviews.llvm.org/D157684 Files: clang/lib/AST/ASTImporter.cpp clang/unittests/AS

[PATCH] D157691: [ASTImporter] Remove extranous FunctionTemplateDecl introduced by templated friend

danix800 created this revision. danix800 added reviewers: balazske, aaron.ballman. danix800 added a project: clang. Herald added subscribers: pengfei, martong, kristof.beyls. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: All. danix800 requested review

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

danix800 added a comment. By `FromContext` dump it's easy to observe that there're two different `FunctionTemplateDecl` (`0x5590cff48048` and `0x5590cff481f8`): |-FriendDecl 0x5590cff48048 col:42 | `-FunctionTemplateDecl 0x5590cff47f80 parent 0x5590cfefeda8 col:42 m | |-... `-FriendD

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

danix800 created this revision. danix800 added reviewers: balazske, aaron.ballman. danix800 added a project: clang. Herald added subscribers: pengfei, martong, kristof.beyls. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: All. danix800 requested review

[PATCH] D157637: [ASTImporter][NFC] Fix typo in testcase

danix800 created this revision. danix800 added a reviewer: balazske. Herald added a subscriber: martong. Herald added a reviewer: a.sidorin. Herald added a project: All. danix800 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix typo in t

[PATCH] D157114: [clang][ASTImporter] Improve StructuralEquivalence algorithm on repeated friends

danix800 updated this revision to Diff 549059. danix800 added a comment. `CXXRecordDecl::friend_iterator` is actually a reversed iterator. Deduplication with different iterator direction produces different result. ASTImporter uses forward iterator so structural equivalence checking should be in

[PATCH] D157584: [clang][Sema] Skip access check on arrays of zero-length element

danix800 created this revision. danix800 added a project: clang. Herald added a project: All. danix800 requested review of this revision. Herald added a subscriber: cfe-commits. Bound check on array of zero-sized element isn't meaningful. Fixes https://github.com/llvm/llvm-project/issues/64564

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 added a comment. I'll finish this patch when CI build succeeds. For future improvement I might start with the idea of marking those generated inner exprs as implicit, this seems to be easier. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157296/new/ https://reviews.llvm.org/D1

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 updated this revision to Diff 548434. danix800 edited the summary of this revision. danix800 added a comment. Cleanup duplicated boilerplate testcase code. Append a few extra child nodes of `CoyieldExpr` dumped in testcase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157296/n

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 added a comment. 1. Use invalid loc: `SourceLocation()` ExprWithCleanups 0x55d1067162c8 'void' `-CoyieldExpr 0x55d106716280 'void' |-CXXMemberCallExpr 0x55d106715ed8 'std::suspend_always':'std::suspend_always' | |-MemberExpr 0x55d106715ea8 '' .yield_value 0x55d10670b220

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 updated this revision to Diff 547843. danix800 added a comment. Use `getEndLoc()` instead of `getBeginLoc()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157296/new/ https://reviews.llvm.org/D157296 Files: clang/lib/Sema/SemaCoroutine

  1   2   >