[PATCH] D149884: [clang][deps] Teach dep directive scanner about _Pragma

benlangmuir created this revision. benlangmuir added reviewers: akyrtzi, Bigcheese, jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. While we cannot handle `_Pragma` used inside macros,

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

benlangmuir added a comment. In D103930#4310061 , @ivanmurashko wrote: > Friendly ping > > @arphaman, @jansvoboda11, I have made the patch buildable on all platforms > and have all tests passed. There was also a small fix (temp path for modules > artef

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

benlangmuir added a comment. We have several new build failures with this change that I'm looking through. So far, a common one is an error of the form /source/module.modulemap: error: redefinition of module /build/Foo.framework/Modules/module.modulemap: note: previously defined here ie. we

[PATCH] D150151: [clang] Prevent creation of new submodules in ASTWriter

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Avoid inferring new submodules for headers in ASTWriter's collection of

[PATCH] D150151: [clang] Prevent creation of new submodules in ASTWriter

benlangmuir added a comment. No test, because I haven't come up with a test case that wouldn't be invalidated by https://reviews.llvm.org/D103930. Let me know if you have any ideas. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150151/new/ https:

[PATCH] D149884: [clang][deps] Teach dep directive scanner about _Pragma

This revision was automatically updated to reflect the committed changes. Closed by commit rGee8ed0b3099e: [clang][deps] Teach dep directive scanner about _Pragma (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149884/new/ h

[PATCH] D150151: [clang] Prevent creation of new submodules in ASTWriter

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5984ea216d2a: [clang] Prevent creation of new submodules in ASTWriter (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D150318: [clang][deps] NFC: Pass around the whole scanning service

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:69 + /// The preprocessing mode used for scanning. + ScanningMode Mode; + /// The output format. Why drop `const`? Com

[PATCH] D150319: [clang][deps] Always use -fmodules-validate-once-per-build-session

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp:26 + ? BuildSessionTimestamp + : std::chrono::system_clock::now().time_since_epoch().count()) { // Initialize targets for object file su

[PATCH] D150320: [clang][deps] Avoid relocatable modules checks

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. With my naming comment addressed, LGTM Comment at: clang/include/clang/Lex/PreprocessorOptions.h:73 + /// Perform extra checks for relocatable modules when loading

[PATCH] D150318: [clang][deps] NFC: Pass around the whole scanning service

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:69 + /// The preprocessing mode used for scanning. + ScanningMode Mode; + /// The output format. jansvoboda11 wrote: > benlangmuir wrote:

[PATCH] D150478: [clang][modules][deps] Parse "FW_Private" module map even after loading "FW" PCM

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM. I ran into a similar issue in a downstream branch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150478/new/ https://reviews.llvm

[PATCH] D150479: [clang][modules][deps] Allow skipping submodule definitions

benlangmuir added inline comments. Comment at: clang/test/ClangScanDeps/modules-private-framework-submodule.c:43 // CHECK-NEXT: "context-hash": "{{.*}}", -// CHECK-NEXT: "module-name": "FW2_Private" +// CHECK-NEXT: "module-name": "FW2"

[PATCH] D150479: [clang][modules][deps] Allow skipping submodule definitions

benlangmuir added inline comments. Comment at: clang/test/ClangScanDeps/modules-private-framework-submodule.c:43 // CHECK-NEXT: "context-hash": "{{.*}}", -// CHECK-NEXT: "module-name": "FW2_Private" +// CHECK-NEXT: "module-name": "FW2"

[PATCH] D148176: [clang][modules] Avoid re-exporting PCH imports on every later module import

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

[PATCH] D148176: [clang][modules] Avoid re-exporting PCH imports on every later module import

benlangmuir added a comment. In D148176#4283942 , @jansvoboda11 wrote: > Can you explain why we need to keep separate `PendingImportedModulesSema` > now? In what situation will it end up aggregating more than one > `PendingImportedModules`? I don't kn

[PATCH] D148176: [clang][modules] Avoid re-exporting PCH imports on every later module import

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe06a91c5996b: [clang][modules] Avoid re-exporting PCH imports on every later module import (authored by benlangmuir). Repository: rG LLVM Github M

[PATCH] D149693: [clang][deps] Make clang-scan-deps write modules in raw format

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, akyrtzi, Bigcheese. Herald added a subscriber: arphaman. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We have no use f

[PATCH] D149693: [clang][deps] Make clang-scan-deps write modules in raw format

benlangmuir updated this revision to Diff 518891. Herald added a subscriber: kadircet. Herald added a project: clang-tools-extra. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149693/new/ https://reviews.llvm.org/D149693 Files: clang-tools-extra/clangd/Compiler.cpp clang/include/clan

[PATCH] D149693: [clang][deps] Make clang-scan-deps write modules in raw format

This revision was automatically updated to reflect the committed changes. Closed by commit rG8fe8d69ddf88: [clang][deps] Make clang-scan-deps write modules in raw format (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149693/

[PATCH] D149777: [clang][deps] Teach dep directive scanner about #pragma clang system_header

benlangmuir created this revision. benlangmuir added a reviewer: akyrtzi. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This ensures we get the correct FileCharacteristic during scanning. In a yet-

[PATCH] D149777: [clang][deps] Teach dep directive scanner about #pragma clang system_header

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7b492d1be0ea: [clang][deps] Teach dep directive scanner about #pragma clang system_header (authored by benlangmuir). Repository: rG LLVM Github Mo

[PATCH] D151855: [clang] Use `{File,Directory}EntryRef` in modular header search (part 2/2)

benlangmuir added a subscriber: JDevlieghere. benlangmuir added a comment. > I think it should be fine to allow dropping the > A.framework/Frameworks/B.framework directory from the reproducer VFS I think technically this is wrong, since if you're missing the symlink, then A might not build -- e

[PATCH] D151855: [clang] Use `{File,Directory}EntryRef` in modular header search (part 2/2)

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. > I don't think it's worth blocking this patch on it, though. What do you think? Agreed, I don't think this is making it worse. Repository: rG LLVM Github Monorepo CHANGES SINCE

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

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. All my comments are addressed, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139052/new/ https://reviews.llvm.org/D139052 ___

[PATCH] D139115: [clang][ExtractAPI] Add support for single symbol SGF

benlangmuir added subscribers: akyrtzi, benlangmuir. benlangmuir added inline comments. Comment at: clang/include/clang-c/Documentation.h:549 +typedef struct CXAPISetImpl *CXAPISet; + @dang please document what this type represents. @akyrtzi what's the preferr

[PATCH] D139115: [clang][ExtractAPI] Add support for single symbol SGF

benlangmuir added a comment. Thanks, the libclang change LGTM other than my comment about the function names. Comment at: clang/include/clang-c/Documentation.h:589 + */ +CINDEX_LINKAGE CXString clang_getSingleSymbolSymbolGraphForUSR(const char *usr, +

[PATCH] D139115: [clang][ExtractAPI] Add support for single symbol SGF

benlangmuir added a comment. libclang changes LGTM, thanks! I'll let the other reviewers who have domain expertise speak for the rest of the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139115/new/ https://reviews.llvm.org/D139115 __

[PATCH] D28299: Module: use PCMCache to manage memory buffers for pcm files.

benlangmuir added a comment. Can we test the already-validated diagnostics? Comment at: include/clang/Basic/FileManager.h:176 + /// Manage memory buffers associated with pcm files. + std::unique_ptr BufferMgr; + Why is this inside the FileManager? It isn't us

[PATCH] D28415: PCH: fix a regression that reports a module is defined in both pch and pcm

benlangmuir added a comment. LGTM https://reviews.llvm.org/D28415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28299: Module: use PCMCache to manage memory buffers for pcm files.

benlangmuir added inline comments. Comment at: include/clang/Basic/FileManager.h:176 + /// Manage memory buffers associated with pcm files. + std::unique_ptr BufferMgr; + manmanren wrote: > benlangmuir wrote: > > Why is this inside the FileManager? It isn't use

[PATCH] D32081: Add support for editor placeholders to Clang's lexer

benlangmuir added a comment. Rather than stick ranges into the diagnostic engine, I think it would be cleaner to have the identifier have a method like `isEditorPlaceholder()` that can be used to avoid situations like this in a principled way, or to customize behaviour for placeholders in the p

[PATCH] D32010: [indexer] Record class template specializations using a new 'SpecializationOf' relationship

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D32010 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D32081: Add support for editor placeholders to Clang's lexer

benlangmuir added a comment. > What do you think about a hybrid approach: Don't suppress diagnostics in the > placeholder range when we handle the placeholders in parser/sema, but do > suppress the range when the placeholder isn't explicitly handled? I'd really rather not plumb this into the di

[PATCH] D32081: Add support for editor placeholders to Clang's lexer

benlangmuir added inline comments. Comment at: include/clang/Basic/IdentifierTable.h:358 + /// Return true if this identifier is an editor placeholder. + bool isEditorPlaceholder() const { Nitpick: There should probably be an example in the doc comment. The

[PATCH] D28299: Module: use PCMCache to manage memory buffers for pcm files.

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Sorry for the delay, I though you were still iterating with @aprantl for some reason. LGTM https://reviews.llvm.org/D28299 ___ cfe-co

[PATCH] D28286: [CodeCompletion] Code complete the missing C++11 keywords

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D28286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D32972: [index] Index simple dependent declaration references

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. A couple of minor comments, but otherwise LGTM. Comment at: include/clang/AST/DeclCXX.h:1569 + bool lookupInBases(BaseMatchesCallback BaseMatches, CXXBasePaths &Pa

[PATCH] D33324: [index] Avoid infinite recursion when looking up a dependent name

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM. One stylistic comment, but I'll leave that up to you. Comment at: lib/AST/CXXInheritance.cpp:286 +BaseRecord = nullptr; + else if (Visi

[PATCH] D118087: Fix running orc-rt tests with LLVM_BUILD_EXTERNAL_COMPILER_RT

benlangmuir created this revision. benlangmuir added reviewers: lhames, fhahn. Herald added a subscriber: mgorny. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add missing dependency on llvm-jitlink when building compiler-rt w

[PATCH] D118087: Fix running orc-rt tests with LLVM_BUILD_EXTERNAL_COMPILER_RT

benlangmuir added a comment. > Does check-orc-rt need to be added to COMPILER_RT_TEST_SUITES too? I guess > not, otherwise we wouldn't have seen this missing dependency in the first > place? It doesn't affect `check-compiler-rt`, because `check-compiler-rt` actually wraps the `check-all` targe

[PATCH] D118087: Fix running orc-rt tests with LLVM_BUILD_EXTERNAL_COMPILER_RT

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0e5ea403e8de: Fix running orc-rt tests with LLVM_BUILD_EXTERNAL_COMPILER_RT (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D101645: [clang] RecursiveASTVisitor visits ObjCPropertyRefExpr's class receiver

benlangmuir added a subscriber: akyrtzi. benlangmuir added inline comments. Comment at: clang/test/Index/Core/index-source.m:410 // CHECK-NEXT: RelCont | classReceivers | c:@F@classReceivers +// CHECK: [[@LINE-3]]:3 | class/ObjC | ClassReceivers | c:objc(cs)ClassReceivers | _OB

[PATCH] D116659: [llvm][clang][vfs] NFC: Simplify directory iteration

benlangmuir added a comment. > I don't know, I'm a bit skeptical we want to make it so easy to ignore errors > so easily. I'd rather require clients to explicitly ignore the error. That's my gut feeling too. I suspect a bunch of this code should be explicitly ignoring `ENOENT`, but not ignoring

[PATCH] D116174: [clang] support relative roots to vfs overlays

benlangmuir added a comment. Each VFS may have its own working directory, so it could be surprising if we use the OS working directory instead of that. This is complicated by the fact the VFS working directory may not be set yet during parsing the yaml (I haven't checked). I'm not really sure

[PATCH] D116174: [clang] support relative roots to vfs overlays

benlangmuir added a comment. Make sure the test failure gets fixed, but otherwise LGTM. > x64 debian > LLVM-Unit.Support/_/SupportTests::VFSFromYAMLTest.RelativePaths Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116174/new/ https://reviews.llvm.o

[PATCH] D99758: [index] Improve macro indexing support

benlangmuir created this revision. benlangmuir added reviewers: akyrtzi, sammccall. Herald added a subscriber: arphaman. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The major change here is to index macro occurrences in more

[PATCH] D99758: [index] Improve macro indexing support

benlangmuir updated this revision to Diff 334808. benlangmuir added a comment. Fixed clang-format issue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99758/new/ https://reviews.llvm.org/D99758 Files: clang/include/clang/Index/DeclOccurrence.h clang/include/clang/Index/IndexingOpti

[PATCH] D99758: [index] Improve macro indexing support

benlangmuir updated this revision to Diff 334820. benlangmuir added a comment. Fix the incomplete type issue with PointerUnion in DeclOccurrence. h. I'm not actually sure how that code compiles without error with other versions of clang. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99

[PATCH] D99758: [index] Improve macro indexing support

benlangmuir updated this revision to Diff 334837. benlangmuir added a comment. Fix clang-tidy warnings CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99758/new/ https://reviews.llvm.org/D99758 Files: clang/include/clang/Index/DeclOccurrence.h clang/include/clang/Index/IndexingOption

[PATCH] D99758: [index] Improve macro indexing support

This revision was automatically updated to reflect the committed changes. Closed by commit rG93c87fc06eca: [index] Improve macro indexing support (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99758/new/ https://reviews.llv

[PATCH] D112229: Reapply [ORC-RT] Configure the ORC runtime for more architectures and platforms

benlangmuir created this revision. benlangmuir added a reviewer: lhames. Herald added subscribers: kristof.beyls, mgorny. benlangmuir requested review of this revision. Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits. Reapply 5692ed0cce8c95

[PATCH] D112229: Reapply [ORC-RT] Configure the ORC runtime for more architectures and platforms

benlangmuir added inline comments. Comment at: clang/cmake/caches/CrossWinToARMLinux.cmake:104 set(COMPILER_RT_BUILD_CRT OFF CACHE BOOL "") +set(COMPILER_RT_BUILD_ORC OFF CACHE BOOL "") set(COMPILER_RT_DEFAULT_TARGET_ONLY ON CACHE BOO

[PATCH] D112229: Reapply [ORC-RT] Configure the ORC runtime for more architectures and platforms

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb8da59475076: Reapply [ORC-RT] Configure the ORC runtime for more architectures and platforms (authored by benlangmuir). Repository: rG LLVM Githu

<    1   2   3   4