[PATCH] D125061: [clang] A more robust way to attach comments

akyrtzi added a comment. Since the issue is specific to enumerators I would recommend against increasing the size of `DeclBase`, which would increase the size of every single `Decl` in the AST. There are multiple ways to approach this: - Consider whether bringing back the comma check just for

[PATCH] D129524: [Lex/HeaderSearch] Only lookup `HeaderFileInfo` for a `FileEntry` from `ExternalSource` once

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Without marking the `HeaderFileInfo` as resolved the `FileEntries` of files that were not found in `ExternalSource` are loo

[PATCH] D129569: [clang/ios] Make -mios-version-min the canonical spelling over -miphoneos-version-min

akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129569/new/ https://reviews.llvm.org/D129569 ___ cfe-commits mailing list cfe-c

[PATCH] D129912: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool`

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Also include a unit test to validate that the `vfs::FileSystem` object is properly used. Repository: rG LLVM Github Mon

[PATCH] D129912: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfbbabd4ca06a: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to… (authored by akyrtzi). Repository: rG LLVM Github Monorep

[PATCH] D129912: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool`

akyrtzi added a comment. In D129912#3661181 , @hubert.reinterpretcast wrote: > The added test is not passing on the AIX builder: > https://lab.llvm.org/buildbot/#/builders/214/builds/2388/steps/6/logs/FAIL__Clang-Unit__83 > > Note that Clang on that pla

[PATCH] D130443: [CGDebugInfo] Access the current working directory from the `VFS`

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. ...instead of calling `llvm::sys::fs::current_path()` directly. Repository: rG LLVM Github Monorepo https://reviews.llv

[PATCH] D125484: [Tooling/DependencyScanning] Rename refactorings towards transitioning dependency scanning to use pre-lexed preprocessor directive tokens

akyrtzi created this revision. Herald added a subscriber: mgorny. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is 1/4 of a series of patches for making the special lexing for dependency scanning

[PATCH] D125486: [Tooling/DependencyScanning] Remove `ExcludedPreprocessorDirectiveSkipMapping` and related functionality

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Depends on D125484 This is 2/4 of a series of patches for making the special lexing for

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Depends on D125486 This is 3/4 of a series of patches for making the special lexing for

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Depends on D125487 This is 4/4 of a series of patches, bringing the following benefits:

[PATCH] D124687: [Tooling/DependencyScanning & Preprocessor] Refactor dependency scanning to record and use pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi added a comment. I've split this in smaller patches: https://reviews.llvm.org/D125484 - NFC rename refactorings https://reviews.llvm.org/D125486 - Remove `ExcludedPreprocessorDirectiveSkipMapping` https://reviews.llvm.org/D125487 - Change to producing pre-lexed directive tokens instead

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

akyrtzi updated this revision to Diff 429033. akyrtzi added a comment. Added #include "clang/Basic/FileEntry.h" in `PreprocessorOptions.h` to accommodate the modules build. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125488/new/ https://revie

[PATCH] D125484: [Tooling/DependencyScanning] Rename refactorings towards transitioning dependency scanning to use pre-lexed preprocessor directive tokens

akyrtzi updated this revision to Diff 429087. akyrtzi added a comment. Rename the `clang-scan-deps` flag from "-preprocess-directives-scan" to "-preprocess-dependency-directives" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125484/new/ https://re

[PATCH] D125484: [Tooling/DependencyScanning] Rename refactorings towards transitioning dependency scanning to use pre-lexed preprocessor directive tokens

akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:120 +clEnumValN(ScanningMode::DependencyDirectivesScan, + "preprocess-directives-scan", + "The set of dep

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

akyrtzi added a comment. In D125488#3510214 , @dexonsmith wrote: > Is there code in DepFS that can/should be deleted as part of this patch, or > in a follow-up, or is it still around as an option? After these changes, with DepFS we are using its multi-

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

akyrtzi added a comment. In D125488#3510297 , @dexonsmith wrote: > [To be clear, my question was because I don't see this patch deleting the > code path that minimizes / saves-minimized sources. Can/should we delete the > "minimize sources" code path?]

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

akyrtzi added a comment. In D125488#3510329 , @dexonsmith wrote: > Seems unfortunate to have a temporary regression in the commit stack, since > then you can't push incrementally (or bisect). Can the prior patch leave > behind the feature in the Depend

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi updated this revision to Diff 429580. akyrtzi added a comment. Fix issue where an empty '#' in a line was causing the immediately following preprocessor directive to be skipped. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125487/new/ htt

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi updated this revision to Diff 429766. akyrtzi added a comment. Make sure to enable line comments for dependency directive lexing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125487/new/ https://reviews.llvm.org/D125487 Files: clang/inc

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi updated this revision to Diff 431150. akyrtzi added a comment. Remove `DependencyScanningFilesystem::disableDirectivesScanning()` function. Unlike source minimization which changes the source contents size and needed to be disabled in certain situations, directive lexing keeps the same co

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

akyrtzi updated this revision to Diff 431151. akyrtzi added a comment. Update due to source change in the previous patch (https://reviews.llvm.org/D125487) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125488/new/ https://reviews.llvm.org/D125488

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp:175 "#define MACRO con tent ", Out)); - EXPECT_STREQ("#define MACRO con tent\n", Out.data()); + EXPECT_STREQ("#define MACRO

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi updated this revision to Diff 431429. akyrtzi marked an inline comment as done. akyrtzi added a comment. Add documentation comments for a couple of fields of `Scanner` in `DependencyDirectivesScanner.cpp` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/lib/Lex/DependencyDirectivesScanner.cpp:153 + + SmallVector CurDirToks; + SmallVector DirsWithToks; jansvoboda11 wrote: > Can you add a comment explaining the relationshi

[PATCH] D124687: [Tooling/DependencyScanning & Preprocessor] Refactor dependency scanning to record and use pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi abandoned this revision. akyrtzi added a comment. This has been superseded by the above set of patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124687/new/ https://reviews.llvm.org/D124687

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

akyrtzi added a comment. Thank you @jansvoboda11 for reviewing and helping me qualify the changes! 🙇🏻‍♂️ If I receive no further comments I will commit the changes on Thursday; if anyone wants some more time to take a look just let me know. I intend to commit in 2 commits, the first patch as one

[PATCH] D91580: [Frontend] Add flag to allow PCM generation despite compiler errors

akyrtzi added a comment. I'd like if we only had to use one flag (`-fallow-pcm-with-compiler-errors`) and have it handle both modules and PCH. Could you make the flag also work for PCH and/or add a test that verifies it works? You may only have to change Opts.AllowPCHWithCompilerErrors = Args

[PATCH] D91580: [Frontend] Add flag to allow PCM generation despite compiler errors

This revision was automatically updated to reflect the committed changes. Closed by commit rG5834996fefc9: [Frontend] Add flag to allow PCM generation despite compiler errors (authored by bnbarham, committed by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D89024: [AST] Fix crashes caused by redeclarations in hidden prototypes

akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. Good catch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89024/new/ https://reviews.llvm.org/D89024

[PATCH] D89024: [AST] Fix crashes caused by redeclarations in hidden prototypes

This revision was automatically updated to reflect the committed changes. Closed by commit rGfbb499ef255b: [AST] Fix crashes caused by redeclarations in hidden prototypes (authored by bnbarham, committed by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D89453: Fix hidden-redecls.m test for some environments

akyrtzi added a comment. Thank you! Are you able to commit it by yourself? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89453/new/ https://reviews.llvm.org/D89453 ___ cfe-commits mailing list cfe-commit

[PATCH] D60193: [OpenCL] Added addrspace_cast operator

akyrtzi added inline comments. Herald added a subscriber: dexonsmith. Comment at: clang/include/clang-c/Index.h:2057 + */ + CXCursor_CXXAddrspaceCastExpr = 129, + Hi Anastasia, apologies for not catching this earlier, but libclang is intended to keep a stable

[PATCH] D102614: [index] Add support for type of pointers to class members

akyrtzi added inline comments. Comment at: clang/lib/Index/USRGeneration.cpp:897 +if (const MemberPointerType *MPT = T->getAs()) { + VisitType(QualType(MPT->getClass(), 0)); + Out << "::*"; A bit better to do `VisitTagDecl(MPT->getClass())`, what do

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

akyrtzi created this revision. Herald added subscribers: dang, arphaman. Herald added a reviewer: jansvoboda11. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This addresses an issue with how the PCH preable works, specifically: 1

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

akyrtzi updated this revision to Diff 318314. akyrtzi added a comment. clang-format changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95159/new/ https://reviews.llvm.org/D95159 Files: clang/include/clang/Driver/Options.td clang/include/cla

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

akyrtzi updated this revision to Diff 318333. akyrtzi added a comment. Use `getValueOr` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95159/new/ https://reviews.llvm.org/D95159 Files: clang/include/clang/Driver/Options.td clang/include/clang/F

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

akyrtzi updated this revision to Diff 318335. akyrtzi edited the summary of this revision. akyrtzi added a comment. Fix typo in commit message, 'state' -> 'stale' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95159/new/ https://reviews.llvm.org/D95

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

This revision was automatically updated to reflect the committed changes. Closed by commit rGb0e89906f5b7: [ASTReader] Allow controlling separately whether validation should be disabled… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D95989: [ASTReader] Always rebuild a cached module that has errors

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa2c1054c303f: [ASTReader] Always rebuild a cached module that has errors (authored by bnbarham, committed by akyrtzi). Repository: rG LLVM Github

[PATCH] D96246: Make sure a module file with errors produced via '-fallow-pcm-with-compiler-errors' can be loaded when using implicit modules

akyrtzi created this revision. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. A module with errors would be marked as out-of-date, then the `compilerModule` action would produce it, but due to the error it would be treated as fail

[PATCH] D96246: Make sure a module file with errors produced via '-fallow-pcm-with-compiler-errors' can be loaded when using implicit modules

akyrtzi updated this revision to Diff 322166. akyrtzi added a comment. clang-format change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96246/new/ https://reviews.llvm.org/D96246 Files: clang/include/clang/Serialization/ASTReader.h clang/lib/

[PATCH] D96246: Make sure a module file with errors produced via '-fallow-pcm-with-compiler-errors' can be loaded when using implicit modules

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa8cb39bab04c: Make sure a module file with errors produced via '-fallow-pcm-with-compiler… (authored by akyrtzi). Repository: rG LLVM Github Monor

[PATCH] D97204: [RFC] Clang 64-bit source locations

akyrtzi added a comment. Hi @miyuki, > A major thing worth noting is that 64-bit source locations will > require an ABI breakage in libclang. This patch changes the bit width > in libclang unconditionally, rather than making it configurable. Is it possible to make the libclang change configurabl

[PATCH] D97204: [RFC] Clang 64-bit source locations

akyrtzi added a comment. In D97204#2586111 , @rsmith wrote: > Can we avoid a libclang ABI break if we don't allow the use of 64-bit source > locations for builds with 32-bit pointers? To @rsmith's point, the simplest option may be to avoid building libcl

[PATCH] D112591: [clang] [Objective C] Inclusive language: use objcmt-allowlist-dir-path= instead of objcmt-white-list-dir-path=

akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. Do you intend to rename `clang/test/ARCMT/whitelisted` directory as a follow-up? Otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

akyrtzi created this revision. Herald added a subscriber: mgrang. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In the context of caching clang invocations it is important to emit diagnostics in deter

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

akyrtzi added a comment. In D135118#3833978 , @steven_wu wrote: > LGTM. > > `RemoveDecl` does become more expensive but I don't have better solution. Luckily AFAICT this is not a "hot" function. > I am also wondering if as follow up we should add an opt

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

akyrtzi added inline comments. Comment at: clang/include/clang/AST/DeclObjC.h:1091 virtual void collectPropertiesToImplement(PropertyMap &PM, PropertyDeclOrder &PO) const {} benlangmuir wrote: > Can we use the exi

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

akyrtzi updated this revision to Diff 465241. akyrtzi added a comment. Herald added subscribers: steakhal, martong. Herald added a reviewer: NoQ. Remove `PropertyDeclOrder` parameter from the `collectPropertiesToImplement` functions. This is not necessary with `PropertyMap` becoming a `MapVector`

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/include/clang/AST/DeclObjC.h:1091 virtual void collectPropertiesToImplement(PropertyMap &PM, PropertyDeclOrder &PO) const {} --

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG371883f46dc2: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics (authored by akyrtzi). Repository: rG LLVM Github Monorepo

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

akyrtzi added a comment. In D135118#3839442 , @nikic wrote: > FYI this caused a noticeable compile-time regression (about 0.4% geomean at > `-O0`): > http://llvm-compile-time-tracker.com/compare.php?from=92233159035d1b50face95d886901cf99035bd99&to=37188

[PATCH] D135490: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic

akyrtzi created this revision. Herald added a subscriber: mgrang. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Commit `371883f46dc23f8464cbf578e2d12a4f92e61917` caused a noticeable compile-time regre

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

akyrtzi added a comment. In D135118#3840259 , @akyrtzi wrote: > In D135118#3839442 , @nikic wrote: > >> FYI this caused a noticeable compile-time regression (about 0.4% geomean at >> `-O0`): >> http://llvm-compi

[PATCH] D135490: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic

akyrtzi added a comment. In D135490#3847454 , @steven_wu wrote: > I don't know too much about this. I guess have a DiagReceiver to force the > emitting order is a good thing but it is kind of weird to have this just for > unused warning. > > I am guessi

[PATCH] D135634: [clang][modules] Serialize VFS overlay paths into PCMs

akyrtzi added a comment. This seems fine to me but note that we no longer depend on the functionality that `test/Index/index-module-with-vfs.m` is testing (and not sure anyone else does), so if there is another change affecting it that is more complicated we could consider removing the test.

[PATCH] D135634: [clang][modules] Serialize VFS overlay paths into PCMs

akyrtzi added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4715 + return createVFSFromOverlayFiles(CI.getHeaderSearchOpts().VFSOverlayFiles, + Diags, BaseFS); +} A bit nitpick but I'd suggest changing t

[PATCH] D135490: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic

This revision was automatically updated to reflect the committed changes. Closed by commit rG0456acbfb942: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

akyrtzi added a comment. @nikic, it seems to have recovered (http://llvm-compile-time-tracker.com/compare.php?from=c49cde6467f9bf200640db763152a9dc7f009520&to=0456acbfb942f127359a8defd1b4f1f44420df3e&stat=instructions) let me know if you have concerns. Repository: rG LLVM Github Monorepo CH

[PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

akyrtzi added a comment. Hi @domada, these changes break compilation of clang, with such build error: FAILED: tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/ASTContext.cpp.o In file included from /llvm-project/clang/lib/AST/ASTContext.cpp:81: In file included from /llvm-project/llvm/i

[PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

akyrtzi added a comment. I've reverted this change from `main` branch, let me know if there's anything I can do to help with addressing the build issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141910/new/ https://reviews.llvm.org/D141910 __

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

akyrtzi added a comment. > would be great to test it more in a semantic way if possible Keep in mind that the specific order of the decls doesn't matter for the purposes of this test, what matters is that the order is the same every time for the same input. I personally think that the addition

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

akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. Is it impractical to add a test for this? Otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https://reviews.llvm.org/D

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

akyrtzi added a comment. Yeah, you mainly need more than 32 decls to exceed the small storage of `Scope::DeclSetTy`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https://reviews.llvm.org/D141625 __

[PATCH] D142028: [clang] remove SUBMODULE_TOPHEADER

akyrtzi added a comment. This is useful for functionality, like in an IDE, to show the top headers of a module import for navigational purposes. Is it reasonable to at least have an alternative implementation for it (e.g. from the module headers find the headers that were not included from other

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. rdar://104386604 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D142143 Files: clang/lib/Lex/Lexer.cpp

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

akyrtzi added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:4420 + DepDirectives.front().Tokens[NextDepDirectiveTokenIndex]; + if (BufferStart + NextTok.Offset >= BufferPtr) +break; benlangmuir wrote: > How do we know this will termin

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

akyrtzi updated this revision to Diff 490629. akyrtzi added a comment. Add test case for dependency directive lexing of ill-formed include inside `__has_include` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142143/new/ https://reviews.llvm.org/D1

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:4415 +if (Result.isNot(tok::header_name)) + return true; +// Advance the index of lexed tokens. benlangmuir wrote: > This case is missing a t

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. akyrtzi marked an inline comment as done. Closed by commit rGed6d09dd4ead: [Lex] For dependency directive lexing, angled includes in `__has_include`… (authored by akyrt

[PATCH] D142236: [clang/driver] Add `-gno-modules` as the negative version of `-gmodules`

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D142236 Files: clang/include/clang/Driver/Options

[PATCH] D142238: [clang/CodeGenActionTest] Use the platform's path separator for the `DebugInfoCWDCodeGen` test

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes a failure in some Windows configuration. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D142238 Fi

[PATCH] D142236: [clang/driver] Add `-gno-modules` as the negative version of `-gmodules`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1ff8a687ae15: [clang/driver] Add `-gno-modules` as the negative version of `-gmodules` (authored by akyrtzi). Repository: rG LLVM Github Monorepo

[PATCH] D142238: [clang/CodeGenActionTest] Use the platform's path separator for the `DebugInfoCWDCodeGen` test

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb2b078adc2d0: [clang/CodeGenActionTest] Use the platform's path se

[PATCH] D127251: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks

akyrtzi updated this revision to Diff 436024. akyrtzi added a comment. Remove `// clang-format off` annotation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127251/new/ https://reviews.llvm.org/D127251 Files: clang/include/clang/Lex/Preprocesso

[PATCH] D127251: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfbaa8b9ae5f3: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within… (authored by akyrtzi). Repository: rG LLVM Github Monore

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

akyrtzi added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:514 + BeginPtr = PP.CurLexer->getBufferLocation(); + SkipRangePtr = &PP.RecordedSkippedRanges[BeginPtr]; + if (*SkipRangePtr) { benlangmuir wrote: > Storing this pointer is o

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

akyrtzi updated this revision to Diff 436554. akyrtzi added a comment. Assert that `SkipExcludedConditionalBlock()` is not recursively called. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127379/new/ https://reviews.llvm.org/D127379 Files: clan

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:514 + BeginPtr = PP.CurLexer->getBufferLocation(); + SkipRangePtr = &PP.RecordedSkippedRanges[BeginPtr]; + if (*SkipRangePtr) { akyr

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

akyrtzi updated this revision to Diff 436612. akyrtzi added a comment. Add more comments about the use of `SkippingExcludedConditionalBlock` and move the new `Preprocessor` fields towards the top of the class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf7e19a592842: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly… (authored by akyrtzi). Repository: rG LLVM Github Mo

[PATCH] D125936: [Sema] Relax an assertion in BuildStmtExpr

akyrtzi added a comment. The assertion was assuming "the expression doesn't need cleanups", have you considered adding a test that checks that the destructor of the temporary inside the asm statement is called, to ensure these temporaries are properly handled? Repository: rG LLVM Github Mon

[PATCH] D125484: [Tooling/DependencyScanning] Rename refactorings towards transitioning dependency scanning to use pre-lexed preprocessor directive tokens

This revision was automatically updated to reflect the committed changes. akyrtzi marked an inline comment as done. Closed by commit rGb58a420ff4f9: [Tooling/DependencyScanning] Rename refactorings towards transitioning… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D125486: [Tooling/DependencyScanning] Remove `ExcludedPreprocessorDirectiveSkipMapping` and related functionality

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb4c83a13f664: [Tooling/DependencyScanning & Preprocessor] Refactor dependency scanning to… (authored by akyrtzi). Changed prior to commit: https:/

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

akyrtzi closed this revision. akyrtzi marked an inline comment as done. akyrtzi added a comment. b4c83a13f664582015ea22924b9a0c6290d41f5b Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

akyrtzi closed this revision. akyrtzi added a comment. b4c83a13f664582015ea22924b9a0c6290d41f5b Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125488/new/ https://reviews.llvm.or

[PATCH] D127251: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The `EndLoc` parameter was always unset so no fixit was emitted. But it is also unnecessary for determining the range so we

[PATCH] D127251: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks

akyrtzi updated this revision to Diff 434940. akyrtzi added a comment. No need to add '#' for the fixit since we have the range of the directive identifier to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127251/new/ https://reviews.llvm.org/

[PATCH] D127251: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks

akyrtzi updated this revision to Diff 434959. akyrtzi added a comment. Remove the parameter from documentation comment as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127251/new/ https://reviews.llvm.org/D127251 Files: clang/include/clang

[PATCH] D127251: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks

akyrtzi updated this revision to Diff 434980. akyrtzi added a comment. Disable `clang-format` checks for the test file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127251/new/ https://reviews.llvm.org/D127251 Files: clang/include/clang/Lex/Pre

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This speeds up preprocessing, specifically for preprocessing the clang sources time is reduced by about -36%, using measure

[PATCH] D127251: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks

akyrtzi added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.c:1-2 +// clang-format off + // RUN: %clang_cc1 -fsyntax-only -verify=pre-c2x-cpp2b %s aaron.ballman wrote: > You can remove this -- we don't require test files to be for

[PATCH] D127251: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks

akyrtzi added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.c:1-2 +// clang-format off + // RUN: %clang_cc1 -fsyntax-only -verify=pre-c2x-cpp2b %s akyrtzi wrote: > aaron.ballman wrote: > > You can remove this -- we don't require t

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

akyrtzi added inline comments. Comment at: clang/include/clang/CodeGen/BackendUtil.h:14 #include "llvm/IR/ModuleSummaryIndex.h" +#include "llvm/Support/VirtualFileSystem.h" #include Recommend to forward declare instead of including the header. ==

[PATCH] D39524: [libclang] Add dummy libclang-headers target

akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D39524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D39050: Add index-while-building support to Clang

akyrtzi added a comment. Hey Eric, In https://reviews.llvm.org/D39050#921748, @ioeric wrote: > >> - I think the implementation of the index output logic (e.g. > >> `IndexUnitWriter` and bit format file) can be abstracted away (and split > >> into separate patches) so that you can unit-test the

[PATCH] D39050: Add index-while-building support to Clang

akyrtzi added a comment. > To be honest, I want this functionality to get in as much as you do, and I'm > more than happy to prioritize the code review for it :) But the current patch > size makes the reviewing really hard (e.g. I would never have caught the > BLOCK issues hadn't I tried runnin

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. "But when would you have a completely empty diagnostic message", you ask dear reader? That is when there is an empty "#warn

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

akyrtzi updated this revision to Diff 502201. akyrtzi added a comment. Avoid passing a new `IgnoringDiagConsumer` for the test since it's unused. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145256/new/ https://reviews.llvm.org/D145256 Files: c

<    1   2   3   >