[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir created this revision. benlangmuir added reviewers: yamaguchi, v.g.vassilev, ruiu, teemperor. Herald added a subscriber: cfe-commits. We have a regex that needs to match a tab character in the command output, but on macOS `sed` doesn't support '\t', causing it to split on the 't' char

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir added a comment. In https://reviews.llvm.org/D47273#1109934, @ruiu wrote: > I wonder if you could replace \t with \0x09. At least it works on my machine > which has GNU sed. Unfortunately that doesn't work either on mac :-\ Repository: rC Clang https://reviews.llvm.org/D47273

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir added a comment. In https://reviews.llvm.org/D47273#1109985, @ruiu wrote: > sed -e "$(echo -e 's/\t.*//')" Yep, that works! Is there a reason you prefer that over the more succinct `$'s/\t.*//'`? Repository: rC Clang https://reviews.llvm.org/D47273 __

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir updated this revision to Diff 148278. benlangmuir edited the summary of this revision. benlangmuir added a comment. Thanks for looking up the supported bash version! Updated diff. https://reviews.llvm.org/D47273 Files: utils/bash-autocomplete.sh Index: utils/bash-autocomplete.

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir closed this revision. benlangmuir added a comment. r333202 https://reviews.llvm.org/D47273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

benlangmuir added a comment. I agree with not detecting these during PP-only, but there's nothing wrong with `<#>`. It's either not a placeholder, or it's part of a placeholder like `<#>#>`, which is a placeholder containing the text ">". Similarly, `<##` could be the start of an empty placeh

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

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

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

benlangmuir added a comment. It would be nice if the doc comment for the single file parse mode flag was updated to include this new functionality. Comment at: lib/Lex/PPDirectives.cpp:2709 +CurPPLexer->pushConditionalLevel(IfToken.getLocation(), /*wasskip*/true, +

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

benlangmuir added inline comments. Comment at: lib/Lex/PPDirectives.cpp:2774 +// the directive blocks. +CurPPLexer->pushConditionalLevel(CI.IfLoc, /*wasskip*/false, + /*foundnonskip*/true, /*foundelse*/true); akyrtzi wrot

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

benlangmuir added a comment. Thanks, this looks good to me. I'd appreciate if @klimek could take a quick look though. Comment at: include/clang/Lex/PreprocessorOptions.h:102 + /// in preprocessor directive conditions it causes all blocks to be parsed so + /// that the clien

[PATCH] D41526: [libclang] Avoid builtin trap for __debug parser_crash pragma

benlangmuir added a comment. Did you consider just using a different pragma that triggers this behaviour instead of avoiding the crash? I don't really have a strong preference but I'd be interested to hear what you think the pros/cons are. Comment at: lib/Parse/ParseDecl.cpp

[PATCH] D44652: [vfs] Don't bail out after a missing -ivfsoverlay file

benlangmuir created this revision. benlangmuir added reviewers: bruno, vsapsai. Herald added a subscriber: cfe-commits. This make -ivfsoverlay behave more like other fatal errors (e.g. missing -include file) by skipping the missing file instead of bailing out of the whole compilation. This makes i

[PATCH] D54529: [clangd] Add USR to textDocument/definition response

benlangmuir added a comment. @sammccall RE using USRs: we want to integrate clangd into a larger source tooling system that is already using USRs extensively to identify symbols. The most obvious case is that we have an index outside of clangd that uses USRs from clang-the-compiler, so exposing

[PATCH] D58924: Replace clang::FileData with llvm::vfs::Status

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM, although I made a small suggestion for clarity. FYI `InPCH` was used by PTH, which was removed a couple of months ago. Comment at: clang/lib/Basic/FileManag

[PATCH] D58924: Replace clang::FileData with llvm::vfs::Status

benlangmuir added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:305 +UFE = &UniqueRealFiles[Status.getUniqueID()]; +Status = llvm::vfs::Status( + Status.getName(), Status.getUniqueID(), harlanhaskins wrote: > benlangmuir wrote: > > Why

[PATCH] D60735: [FileSystemStatCache] Return std::error_code from stat cache methods

benlangmuir added a comment. LGTM, but I'd appreciate someone who has worked on this more recently taking a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60735/new/ https://reviews.llvm.org/D60735 _

[PATCH] D57965: Clean up ObjCPropertyDecl printing

benlangmuir added subscribers: akyrtzi, benlangmuir. benlangmuir added a comment. @arphaman @akyrtzi could you take a look at this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57965/new/ https://reviews.llvm.org/D57965 ___

[PATCH] D57965: Clean up ObjCPropertyDecl printing

benlangmuir added a comment. > @property(attr, attr2) instead of @property ( attr,attr2 ). The style I see most often is `@property (attr, attr2)`, which is in between those two. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57965/new/ https://reviews.llvm.org/

[PATCH] D34512: Add preliminary Cross Translation Unit support library

benlangmuir added a comment. In https://reviews.llvm.org/D34512#856301, @xazax.hun wrote: > In https://reviews.llvm.org/D34512#856184, @dcoughlin wrote: > > > In either case, the important scenario I think we should support is > > choosing at a call site to a C function the most likely definitio

[PATCH] D48685: [PCH+Modules] Load -fmodule-map-file content before including PCHs

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

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir marked 2 inline comments as done. benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:295 +if (MDC) + MDC->applyDiscoveredDependencies(CI); +LastCC1Arguments = CI.getCC1CommandLine(); -

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf80a0ea76072: [clang][deps] Split translation units into individual -cc1 or other commands (authored by benlangmuir). Repository: rG LLVM Github M

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

benlangmuir added inline comments. Comment at: clang/include/clang/Lex/DependencyDirectivesScanner.h:131 +/// \p dependency_directives_scan::tokens_present_before_eof, otherwise this +/// directive will be ignored. /// Why would you want to print without this? I

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

benlangmuir added inline comments. Comment at: clang/include/clang/Lex/DependencyDirectivesScanner.h:131 +/// \p dependency_directives_scan::tokens_present_before_eof, otherwise this +/// directive will be ignored. /// akyrtzi wrote: > benlangmuir wrote: > > Why

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

benlangmuir added inline comments. Comment at: clang/include/clang/Lex/DependencyDirectivesScanner.h:131 +/// \p dependency_directives_scan::tokens_present_before_eof, otherwise this +/// directive will be ignored. /// benlangmuir wrote: > akyrtzi wrote: > > ben

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. You forgot to remove the ` \param PrintMarkerForTokensBeforeEOF ...` from the doc comment. Otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D133611: [clang] sort additional module maps when serializing

benlangmuir accepted this revision. benlangmuir 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/D133611/new/ https://reviews.llvm.org/D133611 __

[PATCH] D133617: [Clang][ScanDeps] Change multiple-commands.c test to use -fmodules-cache-path on implicit builds

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Thanks for catching this! Comment at: clang/test/ClangScanDeps/multiple-commands.c:11 // RUN: split-file %s %t -// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cd

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

benlangmuir created this revision. benlangmuir added reviewers: bruno, jansvoboda11, arphaman. Herald added a subscriber: delcypher. Herald added a project: All. benlangmuir requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits.

[PATCH] D133617: [Clang][ScanDeps] Change multiple-commands.c test to use -fmodules-cache-path on implicit builds

benlangmuir added a comment. Attempting to not do this again: https://reviews.llvm.org/D133622 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133617/new/ https://reviews.llvm.org/D133617 ___ cfe-commits m

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

This revision was automatically updated to reflect the committed changes. Closed by commit rGd96f526196ac: [clang][test] Disallow using the default module cache path in lit tests (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

benlangmuir added a comment. Reverted due to failure on a bot https://lab.llvm.org/buildbot/#/builders/214/builds/3252 I'm not sure how to deal with missing `env -u`. - We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's interpretation of empty string for this variable. I'm no

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

benlangmuir added a comment. In D133622#3788218 , @bruno wrote: >> I'm not sure how to deal with missing `env -u`. >> >> - We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's >> interpretation of empty string for this variable. I'm not s

[PATCH] D128571: [X86] Support `_Float16` on SSE2 and up

benlangmuir added a comment. This broke some compiler-rt tests on Darwin: https://green.lab.llvm.org/green/job/clang-stage1-RA/29920/ Test Result (3 failures / +3) Builtins-x86_64-darwin.Builtins-x86_64-darwin.extendhfsf2_test.c Builtins-x86_64-darwin.Builtins-x86_64-darwin.truncdfhf2_test.

[PATCH] D128571: [X86] Support `_Float16` on SSE2 and up

benlangmuir added a comment. Reverted in eab2a06f0fde due to the Darwin test failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128571/new/ https://reviews.llvm.org/D12857

[PATCH] D128772: [Lex] Make sure to notify `MultipleIncludeOpt` for "read tokens" during fast dependency directive lexing

benlangmuir added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:4251 DepDirectives.front().Tokens[NextDepDirectiveTokenIndex++]; + if (NextDepDirectiveTokenIndex > 1 || DDTok.Kind != tok::hash) { +// Read something other than a preprocessor directive hash.

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

benlangmuir added inline comments. Comment at: clang/lib/Basic/SourceManager.cpp:1016 +return *Name; return StringRef(); } Just a suggestion: `value_or` can be a nice way to express simple cases like this: ``` getFilename(getFileID(SpellingLoc)).value_o

[PATCH] D129220: [clang] Cleanup ASTContext before output files in crash recovery for modules

benlangmuir created this revision. benlangmuir added reviewers: steven_wu, akyrtzi. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When we recover from a crash in a module compilation thread, we nee

[PATCH] D129220: [clang] Cleanup ASTContext before output files in crash recovery for modules

benlangmuir updated this revision to Diff 442706. benlangmuir added a comment. Updated per out-of-band feedback from @steven_wu : - Added an assert to `clearOutputFiles` that the `ASTConsumer` is removed. Normally this would be done in `FrontendAction::EndSourceFile`. This should help avoid re

[PATCH] D129220: [clang] Cleanup ASTContext before output files in crash recovery for modules

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG67a84ec8105e: [clang] Cleanup ASTContext before output files in crash recovery for modules (authored by benlangmuir). Repository: rG LLVM Github M

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When building modules, override secondary outputs (dependency

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

benlangmuir updated this revision to Diff 443379. benlangmuir added a comment. Updates: - Made lookup of module outputs fallible. Not currently used by clang-scan-deps, but since the expectation is for a build system to provide these settings account for possibility of errors. - Attempt to fix

[PATCH] D123104: [Modules] Use looked-up filename when looking for module maps

benlangmuir accepted this revision. benlangmuir added inline comments. Comment at: clang/include/clang/Lex/HeaderSearch.h:758 + bool IsSystemHeaderDir, + StringRef FileName = ""); This parameter co

[PATCH] D129884: [clang][deps] Include canonical invocation in ContextHash

benlangmuir added a comment. In D129884#3677435 , @jansvoboda11 wrote: > Would it make sense for this to replace the existing strict context hash > implementation? It's not clear to me whether this would be a good tradeoff or not: the explicit build c

[PATCH] D129884: [clang][deps] Include canonical invocation in ContextHash

benlangmuir updated this revision to Diff 448370. benlangmuir added a comment. - Added comments to tests - Added a missing test that I accidentally deleted - Rebased on latest main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129884/new/ https://r

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

benlangmuir added a comment. Is the functionality provided by `ORIGINAL_PCH_DIR` still useful for making it possible to move a PCH and its referenced headers? It's not completely clear to me when this feature is used in practice. It would be nice to remove it or change the default behaviour i

[PATCH] D129884: [clang][deps] Include canonical invocation in ContextHash

This revision was automatically updated to reflect the committed changes. Closed by commit rG028717014002: [clang][deps] Include canonical invocation in ContextHash (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129884/new/

[PATCH] D130934: [clang] Update code that assumes FileEntry::getName is absolute NFC

benlangmuir created this revision. benlangmuir added reviewers: bnbarham, jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscriber: cfe-commits. It's an accident that we started return asbol

[PATCH] D130935: [clang] Only modify FileEntryRef names that are externally remapped

benlangmuir created this revision. benlangmuir added reviewers: bnbarham, jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As progress towards having FileEntryRef contain the requested n

[PATCH] D130935: [clang] Only modify FileEntryRef names that are externally remapped

benlangmuir added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:277-278 - if (Status.getName() == Filename) { -// The name matches. Set the FileEntry. + if (Status.getName() == Filename || !Status.ExposesExternalVFSPath) { +// Use the requested name. Se

[PATCH] D130935: [clang] Only modify FileEntryRef names that are externally remapped

benlangmuir updated this revision to Diff 449124. benlangmuir added a comment. Flip the order of comparisons to avoid unnecessary string comparisons. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130935/new/ https://reviews.llvm.org/D130935 Files:

[PATCH] D130934: [clang] Update code that assumes FileEntry::getName is absolute NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rGb4c6dc2e6637: [clang] Update code that assumes FileEntry::getName is absolute NFC (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[PATCH] D130935: [clang] Only modify FileEntryRef names that are externally remapped

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG98cf745a032e: [clang] Only modify FileEntryRef names that are externally remapped (authored by benlangmuir). Repository: rG LLVM Github Monorepo

[PATCH] D131004: [clang] Add FileEntryRef::getNameAsRequested()

benlangmuir created this revision. benlangmuir added a reviewer: bnbarham. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As progress towards having FileManager::getFileRef() return the path as-req

[PATCH] D131017: Fix use-after-free in clang-apply-replacements

benlangmuir created this revision. benlangmuir added reviewers: bnbarham, fmayer. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Accidentally introduced a dangling StringRef in b4c6dc2e6

[PATCH] D130934: [clang] Update code that assumes FileEntry::getName is absolute NFC

benlangmuir added a comment. @fmayer thanks, fix is: https://reviews.llvm.org/D131017 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130934/new/ https://reviews.llvm.org/D130934 ___ cfe-commits mailing li

[PATCH] D131017: Fix use-after-free in clang-apply-replacements

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG54110b8aa010: Fix use-after-free in clang-apply-replacements (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D131076: [clang][modules] Don't depend on sharing FileManager during module build

benlangmuir created this revision. benlangmuir added a reviewer: bnbarham. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Sharing the FileManager between the importer and the module build should onl

[PATCH] D131004: [clang] Add FileEntryRef::getNameAsRequested()

This revision was automatically updated to reflect the committed changes. Closed by commit rG6a79e2ff1989: [clang] Add FileEntryRef::getNameAsRequested() (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131004/new/ https://re

[PATCH] D131076: [clang][modules] Don't depend on sharing FileManager during module build

benlangmuir updated this revision to Diff 449781. benlangmuir added a comment. - Updates for review feedback - Attempt to fix one of the Windows path issues - I'm just guessing based on what other VFS tests are doing. I don't know what the other windows failure is about, and probably need to de

[PATCH] D131076: [clang][modules] Don't depend on sharing FileManager during module build

benlangmuir updated this revision to Diff 450313. benlangmuir added a comment. Herald added a subscriber: ormris. Attempt to fix test failure seen on Windows. It revealed two bugs - Avoid reusing the FileManager at the top-level in clang-scan-deps to make the test behave as expected - Make the F

[PATCH] D131273: [clang] Fix redirection behaviour for cached FileEntryRef

benlangmuir created this revision. benlangmuir added a reviewer: bnbarham. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In 6a79e2ff1989b

[PATCH] D131273: [clang] Fix redirection behaviour for cached FileEntryRef

This revision was automatically updated to reflect the committed changes. Closed by commit rGd038bb196c51: [clang] Fix redirection behaviour for cached FileEntryRef (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131273/new/

[PATCH] D131076: [clang][modules] Don't depend on sharing FileManager during module build

benlangmuir updated this revision to Diff 450356. benlangmuir added a comment. Split out the FileManager change into https://reviews.llvm.org/D131273 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131076/new/ https://reviews.llvm.org/D131076 Files: clang/include/clang/Driver/Options.t

[PATCH] D131076: [clang][modules] Don't depend on sharing FileManager during module build

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfb89cc0ddbd9: [clang][modules] Don't depend on sharing FileManager during module build (authored by benlangmuir). Repository: rG LLVM Github Monor

[PATCH] D131412: [clang][deps] Stop sharing FileManager across module builds in scanner

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Sharing the FileManager across implicit module builds currentl

[PATCH] D131420: [clang][deps] Always generate module paths

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM, thanks for cleaning it up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131420/new/ https://reviews.llvm.org/D131420 _

[PATCH] D131412: [clang][deps] Stop sharing FileManager across module builds in scanner

This revision was automatically updated to reflect the committed changes. Closed by commit rG33af4b22f8ea: [clang][deps] Stop sharing FileManager across module builds in scanner (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D123197: Remove a few effectively-unused FileEntry APIs. NFC

benlangmuir added inline comments. Comment at: clang/unittests/Basic/FileEntryTest.cpp:47-52 FileEntryRef addFile(StringRef Name) { -FEs.push_back(std::make_unique()); +const FileEntry *FE = +FM.getVirtualFile(std::to_string(UniqueNameCounter++), 0, 0); re

[PATCH] D123197: Remove a few effectively-unused FileEntry APIs. NFC

benlangmuir added inline comments. Comment at: clang/unittests/Basic/FileEntryTest.cpp:47-52 FileEntryRef addFile(StringRef Name) { -FEs.push_back(std::make_unique()); +const FileEntry *FE = +FM.getVirtualFile(std::to_string(UniqueNameCounter++), 0, 0); re

[PATCH] D124288: [Index] Remove reference to `UnresolvedUsingIfExists`

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Minor suggestion for the test case, but otherwise LGTM. Comment at: clang/test/Index/using_if_exists.cpp:9 +// CHECK: [[@LINE-1]]:11 | using/C++ | foo | c:@UD@foo |

[PATCH] D124635: Frontend: Delete output streams before closing CompilerInstance outputs

benlangmuir added a comment. LGTM, although I made a suggestion about the spelling. > This fixes theoretical bugs related to proxy streams, which may have > cleanups to run in their destructor. E.g., buffer_ostream supports > pwrite() by holding all content in a buffer until destruction time. Th

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

benlangmuir marked an inline comment as done. benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:55 + getCommandLine(llvm::function_ref< + Expected(const ModuleID &)> + Looku

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

benlangmuir updated this revision to Diff 443755. benlangmuir added a comment. Updates per review - Switched to a per-output callback - Removed preserved-args.c test - Removed error handling that I no longer have a real use for - Only request .d and .diag paths if they were enabled in the origina

[PATCH] D129504: [libclang][ObjC] Inherit availability attribute from containing decls or interface decls

benlangmuir added inline comments. Comment at: clang/tools/libclang/CIndex.cpp:8271 +else if (auto *IMD = dyn_cast(D)) + CD = IMD->getCategoryDecl(); +else if (auto *ID = dyn_cast(DC)) So this goes Impl -> Class, and CategoryImpl -> Category. Should

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

This revision was automatically updated to reflect the committed changes. Closed by commit rG6626f6fec3d3: [clang][deps] Override dependency and serialized diag files for modules (authored by benlangmuir). Changed prior to commit: https://reviews.llvm.org/D129389?vs=443755&id=443955#toc Reposi

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:341-342 + !MDC.OriginalInvocation.getDependencyOutputOpts().OutputFile.empty(); + // FIXME: HadSerializedDiagnostics and HadDependencyFile should be included in + //

[PATCH] D129607: [clang][deps] Fix handling of -MT in module command-line

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a subscriber: mgorny. Herald added a project: All. benlangmuir requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Follow-up to 6626f6fec3d3

[PATCH] D129607: [clang][deps] Fix handling of -MT in module command-line

This revision was automatically updated to reflect the committed changes. Closed by commit rG3ce78cbd2392: [clang][deps] Fix handling of -MT in module command-line (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129607/new/

[PATCH] D129884: [clang][deps] Include canonical invocation in ContextHash

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese, akyrtzi. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The "strict context hash" is insufficient to identify

[PATCH] D129504: [libclang][ObjC] Inherit availability attribute from containing decls or interface decls

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM, thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129504/new/ https://reviews.llvm.org/D129504 _

[PATCH] D121733: Clean pathnames in FileManager.

benlangmuir added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:218 + llvm::sys::path::remove_dots(CleanFilename, /*remove_dot_dot=*/false); + Filename = CleanFilename; + ppluzhnikov wrote: > kadircet wrote: > > this is actually breaking the [con

[PATCH] D92480: [llvm] Unify interface of (ThreadSafe)?RefCountedBase

benlangmuir removed a reviewer: benlangmuir. benlangmuir added a comment. Removing myself as reviewer since I no longer work in this area. Someone who is more involved with llvm Support/ADT should review this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D104348: [index] Fix performance regression with indexing macros

benlangmuir created this revision. benlangmuir added a reviewer: arphaman. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When using FileIndexRecord with macros, symbol references can be seen out of source order, which was cau

[PATCH] D104348: [index] Fix performance regression with indexing macros

benlangmuir closed this revision. benlangmuir added a comment. Pushed rG773ad55a393f Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104348/new/ https://reviews.llvm.org/D104348

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

benlangmuir added a comment. (catching up after I was away last week) I agree with @dexonsmith's analysis above. One historical note: > As a heuristic on top of (4), "last-ref" (before this patch) probably gets > the answer the user expects more often than "first-ref" (this patch) does, > whic

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

benlangmuir added a subscriber: bnbarham. benlangmuir added a comment. I think we should deduplicate the vfs overlays if the same ivfsoverlay is specified in both the pcm and the command-line. @bnbarham any concern about overlay vs chaining behaviour here? I remember you looking at that a while

[PATCH] D136717: [clang] Move getenv call for SOURCE_DATE_EPOCH out of frontend NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG211f5af38af1: [clang] Move getenv call for SOURCE_DATE_EPOCH out of frontend NFC (authored by benlangmuir). Herald added a project: clang. Herald add

[PATCH] D136717: [clang] Move getenv call for SOURCE_DATE_EPOCH out of frontend NFC

benlangmuir added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4324 +if (Epoch.getAsInteger(10, V) || V > MaxTimestamp) { Diags.Report(diag::err_fe_invalid_source_date_epoch) << Epoch << MaxTimestamp; benlangmuir wr

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

benlangmuir created this revision. benlangmuir added a reviewer: steven_wu. Herald added a subscriber: hiraditya. Herald added a project: All. benlangmuir requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald added projects: clang, LLVM. Avoid call

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

benlangmuir added inline comments. Comment at: llvm/lib/MC/MCContext.cpp:62 -static cl::opt +static cl::opt AsSecureLogFileName("as-secure-log-file-name", Interestingly, `opt` doesn't have a parser; this code only worked with the default value before. Repos

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

benlangmuir added inline comments. Comment at: llvm/lib/MC/MCContext.cpp:62 -static cl::opt +static cl::opt AsSecureLogFileName("as-secure-log-file-name", steven_wu wrote: > benlangmuir wrote: > > Interestingly, `opt` doesn't have a parser; this code only work

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

benlangmuir updated this revision to Diff 471351. benlangmuir added a comment. Herald added a subscriber: ormris. - Updated to use cc1 option in clang - Moved the llvm-mc option to a central location so it can be used when initialized MCTargetOptions - Added tests to cover cc1 and cc1as since eac

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

benlangmuir updated this revision to Diff 471592. benlangmuir added a comment. - --target=, remove -o per review - Use "log_file" as path to try to avoid Windows path issue in driver test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136888/new/ https://reviews.llvm.org/D136888 Files:

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

benlangmuir added inline comments. Comment at: llvm/lib/MC/MCContext.cpp:77 AutoReset(DoAutoReset), TargetOptions(TargetOpts) { - SecureLogFile = AsSecureLogFileName; + SecureLogFile = TargetOptions ? TargetOptions->AsSecureLogFile : ""; MaskRay wrote:

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

benlangmuir added inline comments. Comment at: llvm/lib/MC/MCContext.cpp:77 AutoReset(DoAutoReset), TargetOptions(TargetOpts) { - SecureLogFile = AsSecureLogFileName; + SecureLogFile = TargetOptions ? TargetOptions->AsSecureLogFile : ""; benlangmuir wr

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe1f998302276: Move getenv for AS_SECURE_LOG_FILE to clang (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

benlangmuir added a comment. In D135220#3899674 , @dexonsmith wrote: > In D135220#3898849 , @hans wrote: > >> Relatedly, we ran into a problem with `clang-cl /showIncludes` not including >> all files in its outp

[PATCH] D131796: [clang][deps] Use a cc1 invocation in FullDependencies::getCommandLine()

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Instead of trying to "fix" the original driver invocatin by ap

[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately

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. Instead of delaying the generation of command-lines to after all depende

  1   2   3   4   >