[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2221866 , @aeubanks wrote: > This seems to be breaking determinism on Windows builds, see > https://crbug.com/1117026. Can this be reverted and fixed? Will do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D86622: Fix failing tests after VCTOOLSDIR change

aganea added a comment. I've pushed one more fix: rG33ce275fc156c8b015acfad918937028b2cc235c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86622/new/

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added subscribers: amccarth, dblaikie. aganea added a comment. In D85998#2231001 , @zahen wrote: > The build system strives to be deterministic When you say build system, you mean MSBuild? Other? For example, Fastbuild purposely calls

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added a comment. Could you please explain a little bit more what motivated this change? You can achieve almost the same with `env VCToolsInstallDir=F:\Your_Path\ clang-cl ...`. You would also want to pass `-fmsc-version=...` to avoid extracting the version from `cl.exe`. If you're

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added inline comments. Comment at: clang/test/Driver/cl-options.c:685 +// vctoolsdir is handled by the driver; just check that we don't error. Pass -c because fakedir isn't a real toolchain path +// RUN: %clang_cl -c -vctoolsdir fakedir -- %s 2>&1 One

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:73 + MSVCToolChain::ToolsetLayout ) { + // Don't validate the input; trust the value supplied by the user. + // The primary motivation is to prevent unnecessary file and

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added a comment. Thanks, one more minor thing and it should be good to go. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:764 // Check the environment first, since that's probably the user telling us // what they want to use. Please update

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

aganea added a comment. In D87187#2291806 , @dmantipov wrote: > IIUC compiler driver is not intended to be multithreaded. But OK, here is the > version with llvm::call_once(...). Thanks! There could be at least two cases I see, where the driver

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf2efb5742cc9: [LLD][COFF] Cover usage of LLD-as-a-library in tests (authored by aganea). Changed prior to commit:

[PATCH] D87901: [Driver] Filter out /gcc and /gcc-cross if they do not exists

aganea added a comment. Thanks for the patch! I'll let others review it, but I was just being curious: what tool do you use for intercepting & printing `stat(` and `openat(`? How can I repro locally? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

aganea accepted this revision. aganea added a comment. Makes sense, you cache the commonly taken path, but not the (very uncommon) VFS codepath. LGTM, with some minor comments. Thanks again! Comment at: clang/include/clang/Driver/Distro.h:117 - bool IsOpenSUSE() const { -

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. I'm also in favor, I think this is good direction ahead. It'd be nice if following issues were fixed -- in subsequent patches if you wish: - Stage1 `ninja check-scudo` fails many tests for me, see F13037612: errors.txt . - Stage2

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

aganea added inline comments. Comment at: lld/ELF/Driver.cpp:895 const char *argv[] = {config->progName.data(), opt.data()}; + cl::ResetAllOptionOccurrences(); if (cl::ParseCommandLineOptions(2, argv, "", )) ayrivera wrote: > MaskRay wrote: > > ayrivera

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

aganea added a subscriber: rnk. aganea added a comment. Please install & run `git clang-format` before uploading the patch. Comment at: clang/include/clang/Driver/Distro.h:27 +// Special value means that no detection was performed yet. +UninitializedDistro = -1,

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

aganea added a comment. In D69778#2035805 , @llunak wrote: > @aganea Have you tried how this version of the patch affects PR44953? If not, > could you please try? I've applied the patch, I don't see the issue raised in PR44953 anymore. However as

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

aganea updated this revision to Diff 266989. aganea added a comment. Herald added subscribers: cfe-commits, steven_wu, aheejin, arichardson, sbc100, emaste. Herald added a reviewer: espindola. Herald added a project: clang. I'm taking over this patch, as discussed offline with Zachary. I've

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

aganea added a comment. In D43002#2061162 , @echristo wrote: > First question: > > Since split dwarf has to do some similar things can we not use the same > support? This seems to be a lot of changes for this. > > Second question: > > and if we can't use

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

aganea updated this revision to Diff 267111. aganea added a comment. Simplified. Now using `CodeGenOptions` to pass the object filename around. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43002/new/ https://reviews.llvm.org/D43002 Files:

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea created this revision. aganea added reviewers: hans, amccarth, thakis, mstorsjo, akhuang. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. This patch adds some missing information to the `LF_BUILDINFO` which allows for rebuilding a .CPP

[PATCH] D80454: [Clang][test] fix tests when using external assembler

aganea added a comment. Could you please attach a test to demonstrate the issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80454/new/ https://reviews.llvm.org/D80454 ___ cfe-commits mailing list

[PATCH] D80454: [Clang][test] fix tests when using external assembler

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM. Are you planning on adding another patch for the change that you've sent initially? Ideally, I'd like to relax a bit that constraint in `Driver.cpp`, as in D74447

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea marked 2 inline comments as done. aganea added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:320 + /// Executable and command-line used to create a given CompilerInvocation. + const char *BuildTool = nullptr; + ArrayRef CommandLineArgs;

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/test/CodeGen/debug-info-codeview-buildinfo.c:3 +// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s + +int main() { return 42; } Is there a way to launch an arbitrary

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea updated this revision to Diff 267880. aganea marked an inline comment as done. aganea added a comment. As requested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/ https://reviews.llvm.org/D80833 Files:

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a subscriber: maniccoder. aganea added a comment. In D86694#2274682 , @cryptoad wrote: > In D86694#2274548 , @russell.gallop > wrote: > >> I guess using scudo as a general purpose allocator that could

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. In D86694#2277456 , @cryptoad wrote: > I didn't try to make the Exclusive version work, mostly because I was using > the Windows TLS API and the Shared fit right in with those, but it would get > rid of a lot of the contention.

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2109172 , @uweigand wrote: > Hmm, with clang-cl it seems the driver is trying to use this: > Target: s390x-pc-windows-msvc > which of course doesn't exist. Not sure what is supposed to be happening > here, but it

[PATCH] D78903: [Driver] Add option -fproc-stat-report

aganea added inline comments. Comment at: clang/lib/Driver/Driver.cpp:3782 + = Cmd.getProcessStatistics(); + if (ProcStat) { +if (PrintProcessStat) { sepavloff wrote: > aganea wrote: > > In the case where `!ProcStat`, I am wondering if we

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

This revision was automatically updated to reflect the committed changes. Closed by commit rG403f9537924b: [CodeView] Add full repro to LF_BUILDINFO record (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D80833?vs=268906=271698#toc Repository: rG LLVM Github Monorepo

[PATCH] D78903: [Driver] Add option -fproc-stat-report

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. Thanks again for the changes @sepavloff! Looks good to me! You might wait a bit before commiting, if there are further comments. Comment at: clang/lib/Driver/Driver.cpp:3806

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. Thanks for taking the time @hans, @amccarth! I've split this into several smaller patches, to ease bisect if needs be: rGa45409d8855a1e4538990507ef25e9b51c090193 - [Clang] Move clang::Job::printArg to

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

aganea added a comment. But that won't work when compiling & crashing with `-fno-integrated-cc1`, would it? (or if building with `cmake ... -DCLANG_SPAWN_CC1=1`). In that case, normal crashes (not -gen-reproducer) won't go through the `CrashRecoveryContext`. Try running: $

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2101636 , @thakis wrote: > This is still broken; bots have been red for a few hours now. Can we revert > and analyze async, to keep the bots green please? Yes I will revert, but I don't understand. All the tests pass

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2101690 , @thakis wrote: > Hm http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/16533 is > happy so it's not broken on all Windows machines. Guess I'll take a look on > what's going on. You can hold off on

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea marked 2 inline comments as done. aganea added a subscriber: uweigand. aganea added inline comments. Comment at: clang/test/CodeGen/debug-info-codeview-buildinfo.c:4 +// RUN: %clang_cl /c /Z7 /Fo%t.obj -fdebug-compilation-dir . -- %s +// RUN: llvm-pdbutil dump --types

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea updated this revision to Diff 272556. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/ https://reviews.llvm.org/D80833 Files: clang/test/CodeGen/debug-info-codeview-buildinfo.c lld/COFF/PDB.cpp

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2108423 , @uweigand wrote: > > Line 4 here fails on s390x but not on other Unix flavors, see: > >

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

aganea marked an inline comment as done. aganea added inline comments. Comment at: clang-tools-extra/clangd/index/Background.cpp:154 assert(this->IndexStorageFactory && "Storage factory can not be null!"); - for (unsigned I = 0; I < ThreadPoolSize; ++I) { + for (unsigned I

[PATCH] D82352: [clangd] Make background index thread count calculation clearer

aganea added a comment. LGTM. Thanks for reverting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82352/new/ https://reviews.llvm.org/D82352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. When using `clang-cl`, this code should set it to the right format, on any platform: https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/Driver.cpp#L1071 However maybe something else happens on big endian architectures? @uweigand Would you mind running the

[PATCH] D78903: [Driver] Add option -fproc-stat-report

aganea added inline comments. Comment at: clang/lib/Driver/Driver.cpp:3782 + = Cmd.getProcessStatistics(); + if (ProcStat) { +if (PrintProcessStat) { In the case where `!ProcStat`, I am wondering if we shouldn't emit zero values, in the

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

aganea added inline comments. Comment at: clang/tools/driver/driver.cpp:518 + CRC.DumpStackAndCleanupOnFailure = true; + CRC.RunSafely([&]() { abort(); }); } The only concern I have is that a unrelated call stack will be printed. Could you

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. Reverted in rG2ae0df5be7408a79524743762b6c74953f31b805 . I'll investigate the issue on my end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2101057 , @thakis wrote: > Looks like this breaks tests on mac: http://45.33.8.238/mac/15751/step_10.txt > > Please take a look. Checking now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea updated this revision to Diff 268906. aganea added a comment. Fix tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/ https://reviews.llvm.org/D80833 Files: clang/include/clang/Basic/CodeGenOptions.h

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea updated this revision to Diff 268881. aganea marked 11 inline comments as done. aganea added a comment. Addressed comments (see inline). Ensured the generated .OBJ contains relative paths when passing `-fdebug-compilation-dir . -no-canonical-prefixes`. Repository: rG LLVM Github

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2073458 , @hans wrote: > a. The compiler path is only absolute because it was absolute when put into > argv[0] right? I don't see anything in the code that makes it absolute? I > imagine most build systems will invoke

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea marked an inline comment as done. aganea added a comment. I didn't change the PWD/CWD stored in `LF_BUILDINFO`, it has already been done by Reid a while ago: D53179 (it already stores absolute paths) However absolute paths are stored by design, if we

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea marked 5 inline comments as done. aganea added a comment. In D80833#2071863 , @hans wrote: > In D80833#2071818 , @aganea wrote: > > > I didn't change the PWD/CWD stored in `LF_BUILDINFO`, it has already been

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea updated this revision to Diff 268359. aganea added a comment. Herald added subscribers: jfb, dexonsmith. - Using `sys::flattenWindowsCommandLine` instead of previous quoting function, as suggested by @hans - Added Clang tests for `-fdebug-compilation-dir .` - Added LLD support for making

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea marked an inline comment as done. aganea added a comment. In D80833#2069246 , @amccarth wrote: > Does the full path to the tool end up only in the object file, or does this > make it all the way to the final executable or library? The

[PATCH] D93772: [Clang][Driver] Fix read-after-free when using /clang:

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3854b81b0fd2: [Clang][Driver] Fix read-after-free when using /clang: (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

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

[PATCH] D93772: [Clang][Driver] Fix read-after-free when using /clang:

aganea created this revision. aganea added reviewers: thakis, hans, rnk, neerajksingh. aganea requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As described in https://bugs.llvm.org/show_bug.cgi?id=42501 Fixes PR42501 Repository: rG

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

aganea updated this revision to Diff 304364. aganea edited the summary of this revision. aganea edited reviewers, added: mstorsjo; removed: reames, espindola. aganea added a comment. Simplify. Added coverage when using `clang-cl` since this is a MSVC-specific feature. When

[PATCH] D83025: [clang] Include type specifiers in typo correction when checking isCXXDeclarationSpecifiers.

aganea added a comment. We're seeing the same issue mentionned in PR45498 , but with the repro below. `git bisect` points to this patch. Could anyone please possibly confirm? Simply build clang with `-DLLVM_ENABLE_ASSERTIONS=ON`. // compile with:

[PATCH] D83025: [clang] Include type specifiers in typo correction when checking isCXXDeclarationSpecifiers.

aganea added a comment. In D83025#2389486 , @hokein wrote: > In D83025#2389140 , @aganea wrote: > >> We're seeing the same issue mentionned in PR45498 >> , but with the

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

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

[PATCH] D78899: [Driver] Add callback to Command execution

aganea accepted this revision. aganea 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/D78899/new/ https://reviews.llvm.org/D78899

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. In D86694#2463429 , @russell.gallop wrote: > - With scudo all lit tests (generally) pass, but there are some intermittent > failures (Access Violation) on the following tests: > >>

[PATCH] D94324: [InitLLVM] Ensure SIGPIPE handler installed before sigaction()

aganea added a comment. What are the exact conditions to repro Nick's failure? On Fedora 32, I ran `py llvm-lit sigpipe-handling.c` before this patch and before rGbf401256edd00e921a5d3a0bf4cf6ee66ae51cd6 , the test suceeds,

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

aganea marked 8 inline comments as done. aganea added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:212 + /// Output filename used in the COFF debug information. + std::string COFFOutputFilename; + rnk wrote: > Let's bikeshed the name

[PATCH] D103773: [clang-cl] Add /permissive and /permissive-

aganea added inline comments. Comment at: clang/test/Driver/cl-permissive.c:15 +// PERMISSIVE-OVERWRITE-NOT: "-fdelayed-template-parsing" +// RUN: %clang_cl /permissive- /Zc:twoPhase- -### -- %s 2>&1 | FileCheck -check-prefix=PERMISSIVE-MINUS-OVERWRITE %s +//

[PATCH] D102339: [clang] On Windows, ignore case and separators when discarding duplicate dependency file paths.

aganea added a comment. In D102339#2755867 , @thakis wrote: > We share obj files built on linux and on windows. So that's a goal for us. Hello @thakis, I believe you don't use this codepath to generate the cache key, do you? I thought you had a custom

[PATCH] D102736: Fix tmp files being left on Windows builds.

aganea added a comment. Do you think the existing crash tests can be modified to validate that .tmp files are deleted indeed? Comment at: clang/lib/Frontend/CompilerInstance.cpp:829 +Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text; +// Use OF_Delete on

[PATCH] D102736: Fix tmp files being left on Windows builds.

aganea added a comment. In D102736#2767516 , @amccarth wrote: > I seem to recall some thrashing on this topic a few months ago. If I'm > remembering correctly, setting the disposition to delete temporary files on > Windows was causing problems with

[PATCH] D102339: [clang] On Windows, ignore case and separators when discarding duplicate dependency file paths.

aganea added inline comments. Comment at: clang/lib/Frontend/DependencyFile.cpp:145 + StringRef SearchPath; +#ifdef _WIN32 + // Make the search insensitive to case and separators. amccarth wrote: > rnk wrote: > > I feel like this should somehow be a property

[PATCH] D102736: Fix tmp files being left on Windows builds.

aganea added inline comments. Comment at: clang/include/clang/Frontend/CompilerInstance.h:170 std::string TempFilename; +Optional TempFile; Can we always use `TempFile`? And remove `TempFilename` in that case? Comment at:

[PATCH] D102633: [clang-scan-deps] Improvements to thread usage

aganea added a subscriber: mehdi_amini. aganea added a comment. In D102633#2769762 , @arphaman wrote: > It might be good for @aganea to take a look as well. Thanks! I actually work with @saudi, I already took a look at the patch before uploading.

[PATCH] D102633: [clang-scan-deps] Improvements to thread usage

aganea added a comment. @dexonsmith Yes, using the Clang Tooling API seems like a good option, however some logic could/would be needed from `clang/tools/clang-scan-deps/ClangScanDeps.cpp`. Using clang-scan-deps as-a-DLL seemed like the best option on the short term, since we're using its

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

aganea added a comment. In D80344#2792355 , @tentzen wrote: > Thank you for reporting this . From the callstack it does not seem related > to this patch. It was actually `git bisect` which pointed to me to this patch, it could be a side-effect of it.

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

aganea added a comment. Thanks for the quick fix! Would you mind fixing the two failing tests please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103664/new/ https://reviews.llvm.org/D103664 ___

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

aganea added a comment. In D80344#2797317 , @tentzen wrote: > @aganea , this patch should be zero-impact without explicit option > -fasync-exceptions. Are you also seeing a crash without this option? I'm using `/EHa` which I expect translates to

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

aganea added a comment. Hello, this patch introduces a regression in one of our codebases. The compilation of several TUs fails with the following callstack: PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

aganea added a comment. A few questions: Does this work on x86 targets? Are the scudo tests below being built with /MD or with /MT? Would this change compile/work on MinGW as well? Comment at: compiler-rt/lib/scudo/scudo_platform.h:72 +#elif SANITIZER_WINDOWS +const uptr

[PATCH] D95534: clang-cl: Invent a /winsysroot concept

aganea added a comment. In D95534#2534510 , @smeenai wrote: > Now if only Windows could case-correct their SDKs so we didn't need VFS > overlays and symlinks for the linker... I opened a ticket

[PATCH] D98824: [Tooling] Handle compilation databases containing commands with double dashes

This revision was automatically updated to reflect the committed changes. Closed by commit rGe030ce3ec790: [Tooling] Handle compilation databases containing commands with double dashes (authored by jnykiel, committed by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D99363: [Windows] Turn off text mode in TableGen and Rewriter to stop CRLF translation

aganea added a comment. Sorry, but after this patch, I still see the issue mentioned in https://reviews.llvm.org/D96363#2650460 @abhina.sreeskantharajan are you be able to confirm the issue on your end? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D96363: Mark output as text if it is really text

aganea added a comment. Hello! This patch breaks compilation when using `-frewrite-includes` on Windows. It adds an extra line with CRLF, which causes compilation failures with macros line-breaks: $ cat -A hello.cpp int foo();^M$ int bar();^M$ #define HELLO \^M$ foo(); \^M$

[PATCH] D99363: [Windows] Turn off text mode in TableGen and Rewriter to stop CRLF translation

aganea added a comment. In D99363#2652907 , @abhina.sreeskantharajan wrote: > In D99363#2652899 , @aganea wrote: > >> Sorry, but after this patch, I still see the issue mentioned in >>

[PATCH] D99363: [Windows] Turn off text mode in TableGen and Rewriter to stop CRLF translation

aganea added a comment. In D99363#2653476 , @abhina.sreeskantharajan wrote: > In D99363#2653201 , @aganea wrote: > >> I'm just wondering if D96363 and all >> attached subsequent

[PATCH] D99363: [Windows] Turn off text mode in TableGen and Rewriter to stop CRLF translation

aganea added a comment. I'm just wondering if D96363 and all attached subsequent patches shouldn't be reverted for now. This is quite trivial case uncovered by tests. On re-land, I would then add a test validating the issue on Windows: $ cat -A

[PATCH] D99363: [Windows] Turn off text mode in TableGen and Rewriter to stop CRLF translation

aganea added a comment. In D99363#2653476 , @abhina.sreeskantharajan wrote: > There were a lot of similar patches so reverting all of them might be more > work than isolating the change that caused it and reverting that. It seems > that the patch you

[PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

aganea added a comment. I am still concerned by the fact that this patch doesn't fix the issue mentionned in https://reviews.llvm.org/D96363#2650460 Was the intention to fix that issue? Will the fix be done in a subsequent patch? Comment at:

[PATCH] D99973: [Windows] Add test coverage for line endings when rewriting includes

This revision was automatically updated to reflect the committed changes. aganea marked an inline comment as done. Closed by commit rG8fbc05acd553: [Windows] Add test coverage for line endings when rewriting includes (authored by aganea). Changed prior to commit:

[PATCH] D99973: [Windows] Add test coverage for line endings when rewriting includes

aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/test/Frontend/rewrite-includes-macros.cpp:15 +} \ No newline at end of file mstorsjo wrote: > I guess the missing newline at end of file isn't one of the aspects that >

[PATCH] D99426: [SystemZ][z/OS][Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

aganea added a comment. In D99426#2666141 , @abhina.sreeskantharajan wrote: > In D99426#2665361 , @aganea wrote: > >> I am still concerned by the fact that this patch doesn't fix the issue >> mentionned in

[PATCH] D99363: [Windows] Turn off text mode in TableGen and Rewriter to stop CRLF translation

aganea added a comment. >> In D99363#2653476 , >> @abhina.sreeskantharajan wrote: >> >>> > > This was only other file from https://reviews.llvm.org/D96363 that was using > OF_TextWithCRLF instead of OF_Text. Please let me know if this latest patch >

[PATCH] D99973: [Windows] Add test coverage for line endings when rewriting includes

aganea created this revision. aganea added reviewers: abhina.sreeskantharajan, rnk, amccarth, MaskRay, mstorsjo. aganea requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Validate that we properly generating a single line ending on Windows

[PATCH] D99837: [Windows] Turn off text mode correctly in Rewriter to stop CRLF translation

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. It works now, thanks! :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99837/new/ https://reviews.llvm.org/D99837

[PATCH] D98824: [Tooling] Handle compilation databases with clang-cl commands generated by CMake 3.19+

aganea added inline comments. Comment at: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp:181 + // ...including when the inputs are passed after -- + if (Opt.matches(OPT__DASH_DASH)) { +continue; You can leave out the braces on a

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

aganea added a comment. Thanks for the update @vvereschaka ! Just a few minor things and it should be good to go. Also please use `git diff -U9` next time to add context to the patch. Comment at: clang/docs/UsersManual.rst:783 It is possible to specify this option

[PATCH] D97138: [clang][flang] Improve the consistency of the code-base

aganea added inline comments. Comment at: clang/tools/driver/driver.cpp:349 "source, and associated run script.\n"); - SmallVector argv(argv_, argv_ + argc_); + SmallVector ArgValues(Argv, Argv + Argc); What about just `Args`?

[PATCH] D97138: [clang][flang] Improve the consistency of the code-base

aganea accepted this revision. aganea added a comment. In D97138#2586988 , @achieveartificialintelligence wrote: > Thanks @aganea. And I have a question that, should I use `argc` & `argv` or > `argC` & `argV` in the flang part now ? Looks good as it is

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM, just a few minor things: Comment at: clang/docs/UsersManual.rst:798 + Setting ``CC_PRINT_PROC_STAT`` to ``1`` enables the feature, the report goes to + the stdout

[PATCH] D92191: [clang-scan-deps] Add support for clang-cl

aganea added a comment. Thanks! We found the issue. I think I'll revert the patch, and will reland a fixed version. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92191/new/ https://reviews.llvm.org/D92191

[PATCH] D92191: [clang-scan-deps] Add support for clang-cl

aganea added a comment. @gulfem We're taking a look now. Could you please post a link to the full build log, including the cmake flags used? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92191/new/ https://reviews.llvm.org/D92191

[PATCH] D100755: [llvm-rc] [3/4] Run clang to preprocess input files

aganea added inline comments. Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:94 + +ErrorOr findClang(const char *Argv0) { + StringRef Parent = llvm::sys::path::parent_path(Argv0); Since you're not using the `std::error_code` below in the call site, should this

[PATCH] D100755: [llvm-rc] [3/4] Run clang to preprocess input files

aganea accepted this revision. aganea added a comment. LGTM. Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:94 + +ErrorOr findClang(const char *Argv0) { + StringRef Parent = llvm::sys::path::parent_path(Argv0); mstorsjo wrote: > aganea wrote: > > Since you're not

[PATCH] D95099: [clang-scan-deps] : Support -- in clang command lines.

This revision was automatically updated to reflect the committed changes. Closed by commit rG488a19d00cba: [clang-scan-deps] Support double-dashes in clang command lines (authored by saudi, committed by aganea). Changed prior to commit: https://reviews.llvm.org/D95099?vs=318304=338329#toc

<    1   2   3   4   5   >