[PATCH] D114034: [clang-tidy] fix debug-only test failure

dstenb added a comment. In D114034#3136057 , @mattbeardsley wrote: > @dstenb - wanted to check with you too to make sure that this change to > pr37091.cpp seems like it won't interfere with the original intent of the > test

[PATCH] D91519: [AST][Mach0] Fix unused-variable warnings

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2d1f471e45af: [Mach0] Fix unused-variable warnings (authored by ehjogab, committed by dstenb). Herald added a project: LLVM. Changed prior to

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

dstenb added a comment. In D73534#1918940 , @dstenb wrote: > In D73534#1918890 , @manojgupta > wrote: > > > Hi, > > > > I see another crash with this change when building gdb. > > > > Reduced test case: > >

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

dstenb added a comment. In D73534#1918890 , @manojgupta wrote: > Hi, > > I see another crash with this change when building gdb. > > Reduced test case: > struct type *a(type *, type *, long, long); > enum b {}; > static int empty_array(type *, int c) {

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

dstenb added a comment. In D73534#1882127 , @djtodoro wrote: > @dstenb No problem, we have already addressed the issue. Thanks to @vsk and > @nickdesaulniers! I'll update the patch. Great, thanks! CHANGES SINCE LAST ACTION

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

dstenb added a comment. In D73534#1881353 , @nickdesaulniers wrote: > As a heads up, Linaro's ToolChain Working Group's Linux kernel CI lit up on > this change. I see it's been reverted (thank you). Please cc me on the > updated patch and I can help

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

dstenb added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:870 if (MI->isCandidateForCallSiteEntry() && -DAG->getTarget().Options.EnableDebugEntryValues) +DAG->getTarget().Options.SupportsDebugEntryValues)

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

dstenb added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:870 if (MI->isCandidateForCallSiteEntry() && -DAG->getTarget().Options.EnableDebugEntryValues) +DAG->getTarget().Options.SupportsDebugEntryValues)

[PATCH] D71508: [DebugInfo] Duplicate file names in debug info

dstenb added a comment. It should be possible to create a test for this using something like: RUN: rm -rf %t && mkdir %t RUN: cp %s %t/filename.c RUN: cd %t RUN: clang ./filename.c [...] Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D58665: [analyzer] Handle comparison between non-default AS symbol and constant

This revision was automatically updated to reflect the committed changes. Closed by commit rC355592: [analyzer] Handle comparison between non-default AS symbol and constant (authored by dstenb, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D58665: [analyzer] Handle comparison between non-default AS symbol and constant

dstenb added a comment. Thanks for the review! I'll submit this shortly then. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58665/new/ https://reviews.llvm.org/D58665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58665: [analyzer] Handle comparison between non-default AS symbol and constant

dstenb updated this revision to Diff 188538. dstenb added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58665/new/ https://reviews.llvm.org/D58665 Files: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp test/Analysis/ptr-cmp-const-trunc.cl Index:

[PATCH] D58665: [analyzer] Handle comparison between non-default AS symbol and constant

dstenb created this revision. dstenb added reviewers: NoQ, zaks.anna, george.karpenkov. Herald added subscribers: cfe-commits, Charusso, jdoerfert, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. When comparing a

[PATCH] D55702: [clangd] Fix memory leak in ClangdTests.

dstenb added a comment. With this patch applied we don't see any issues on our end. Thanks for the help! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55702/new/ https://reviews.llvm.org/D55702 ___

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

dstenb added a comment. Any more comments or concerns, or can I land this? https://reviews.llvm.org/D45686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

This revision was automatically updated to reflect the committed changes. Closed by commit rC333413: Fix emission of phony dependency targets when adding extra deps (authored by dstenb, committed by ). Repository: rL LLVM https://reviews.llvm.org/D44568 Files:

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

This revision was automatically updated to reflect the committed changes. Closed by commit rL333413: Fix emission of phony dependency targets when adding extra deps (authored by dstenb, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

dstenb added a comment. In https://reviews.llvm.org/D44568#1114188, @vsapsai wrote: > Looks good to me. Just watch the build bots in case some of them are strict > with warnings and require `(void)AddFilename(Filename)`. Yes, I'll keep an eye out for that. I'll submit this shortly. Thanks

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

dstenb added inline comments. Comment at: test/Frontend/dependency-gen-extradeps-phony.c:6-7 + +// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra +// CHECK-NOT: .c: +// CHECK: 1.extra: vsapsai wrote: > I think it would be better to have a check > >

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

dstenb updated this revision to Diff 148819. dstenb marked an inline comment as done. dstenb added a comment. Addressed vsapsai's comments. https://reviews.llvm.org/D44568 Files: lib/Frontend/DependencyFile.cpp test/Frontend/dependency-gen-extradeps-phony.c Index:

[PATCH] D47251: Add a lit reproducer for PR37091

dstenb abandoned this revision. dstenb added a comment. Merged into https://reviews.llvm.org/D45686. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47251 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

dstenb updated this revision to Diff 148791. dstenb added a comment. Update patch to include the clang-tools-extra test case originally added in https://reviews.llvm.org/D47251. https://reviews.llvm.org/D45686 Files: cfe/trunk/include/clang/Driver/Compilation.h

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

dstenb updated this revision to Diff 148604. dstenb added a comment. Query TheDriver.isSaveTempsEnabled() at uses instead of storing the value in the constructor. https://reviews.llvm.org/D45686 Files: include/clang/Driver/Compilation.h lib/Driver/Compilation.cpp lib/Driver/Driver.cpp

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

dstenb added inline comments. Comment at: lib/Driver/Compilation.cpp:276-277 + + // Temporary files added by diagnostics should be kept. + SaveTempsEnabled = true; } lebedev.ri wrote: > Is there a test that breaks without this? Yes, the following tests fail:

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

dstenb updated this revision to Diff 148592. dstenb marked an inline comment as done. dstenb added a comment. Renamed SaveTempsEnabled field to KeepTempFiles. https://reviews.llvm.org/D45686 Files: include/clang/Driver/Compilation.h lib/Driver/Compilation.cpp lib/Driver/Driver.cpp

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

dstenb updated this revision to Diff 148577. dstenb retitled this revision from "[Tooling] Clean up tmp files when creating a fixed compilation database" to "[Driver] Clean up tmp files when deleting Compilation objects". dstenb edited the summary of this revision. dstenb added a comment. I

[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

dstenb added inline comments. Comment at: lib/Tooling/CompilationDatabase.cpp:303 + // Remove temp files. + Compilation->CleanupFileList(Compilation->getTempFiles()); + erichkeane wrote: > It seems to me that this would be best called from the destructor of

[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

dstenb added a comment. Ping. We have added a lit reproducer for this now in clang-tools-extra: https://reviews.llvm.org/D47251. Repository: rC Clang https://reviews.llvm.org/D45686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D47251: Add a lit reproducer for PR37091

dstenb created this revision. Herald added a subscriber: cfe-commits. This adds a lit reproducer that verifies that no temporary assembly files are left behind when using clang-tidy with a target that does not support the internal assembler. The fix is in Tooling

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

dstenb added reviewers: bruno, vsapsai. dstenb added subscribers: bruno, vsapsai. dstenb added a comment. @bruno, @vsapsai: I added you since you I saw that you recently reviewed, respectively delivered, https://reviews.llvm.org/D30881. That is the only DependencyFile commit since October;

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

dstenb added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D44568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

dstenb added a comment. Ping. It feels a bit nasty that the tools leave behind temporary files, so I think that it would be good to find a fix for that. Repository: rC Clang https://reviews.llvm.org/D45686 ___ cfe-commits mailing list

[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

dstenb created this revision. dstenb added reviewers: klimek, sepavloff, arphaman. Herald added a subscriber: cfe-commits. In https://reviews.llvm.org/rL327851 the createUniqueFile() and createTemporaryFile() variants that do not return the file descriptors were changed to create empty files,

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

dstenb added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D44568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44774: [Driver] Allow use of -fsyntax-only together with -MJ

dstenb added a comment. Our legacy frontend does not support -MJ, so when using that frontend for code generation, we invoke clang with -MJ, and at the same use -fsyntax-only to get the improved diagnostics that clang provides. This is idiosyncratic and probably hacky, I know, but it works

[PATCH] D44774: [Driver] Allow use of -fsyntax-only together with -MJ

dstenb added a comment. Downstream we use -MJ in a bit of an idiosyncratic way, as we're in a transition period where we, for a subset of the code base, only use the clang frontend for diagnostics, and not for the code generation. However, if you don't think that using -fsyntax-only and -MJ

[PATCH] D44774: [Driver] Allow use of -fsyntax-only together with -MJ

dstenb created this revision. dstenb added reviewers: joerg, klimek, rsmith. Herald added a subscriber: cfe-commits. When using -MJ together with -fsyntax-only, clang would hit an assert in DumpCompilationDatabase() when trying to get the filename for the output field. This patch fixes that by

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

dstenb added a comment. A small caveat with this patch is that it does not fix the case where the input file as also added as an extra dependency with -fdepfile-entry; however, I reasoned that it shouldn't really be a problem in practice. I thought that it was a good trade-off ignoring that

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

dstenb created this revision. dstenb added reviewers: rsmith, pcc, krasin. Herald added a subscriber: cfe-commits. This commit fixes a bug where passing extra dependency entries (using -fdepfile-entry) would result in -MP incorrectly emitting a phony target for the input file, and no phony target