[PATCH] D158279: clang: Make rewrite-includes-macros.cpp runnable on non-Win

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. Sure. This was meant to catch an issue that only happened on Windows but it should work on all platforms. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158279/new/ https://rev

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

aganea added subscribers: thieta, saudi. aganea added a comment. + @saudi @thieta Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147256/new/ https://reviews.llvm.org/D147256 ___ cfe-commits mailing list c

[PATCH] D146490: [Support] On Windows, ensure that UniqueID is really stable

aganea added a comment. Fair enough. There are several choices forward: either we mark the issue as "Will Not Fix" or I can try only scoping this patch to only keep the handle for network drives/paths. Any other suggestions? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D146490: [Support] On Windows, ensure that UniqueID is really stable

aganea updated this revision to Diff 506820. Herald added a subscriber: ormris. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146490/new/ https://reviews.llvm.org/D146490 Files: clang-tools-extra/clangd/unittests/FSTests.cpp clang/include/clang/

[PATCH] D146490: [Support] On Windows, ensure that UniqueID is really stable

aganea created this revision. aganea added reviewers: mstorsjo, hans, kbobyrev, dexonsmith, zero9178. Herald added subscribers: kadircet, arphaman, hiraditya. Herald added a project: All. aganea requested review of this revision. Herald added projects: clang, LLVM, clang-tools-extra. Herald added s

[PATCH] D142224: [Support] Emulate SIGPIPE handling in raw_fd_ostream write for Windows

aganea added inline comments. Comment at: llvm/lib/Support/Windows/Signals.inc:834 + int RetCode = (int)EP->ExceptionRecord->ExceptionCode; + if (RetCode == (0xE000 | EX_IOERR)) +return; erichkeane wrote: > This patch seems to cause a self-build Werror

[PATCH] D115103: Leak Sanitizer port to Windows

aganea added a comment. @clemenswasser We hit this once in a while, when Microsoft update their libraries. But I'm surprised about the CC at the beginning. How can this issue be reproduced? Can you show a disassembly dump from the VS debugger, around the address of the function that fails? CH

[PATCH] D115103: Leak Sanitizer port to Windows

aganea added a comment. In D115103#4097460 , @clemenswasser wrote: > Do you have a suggestion how I could fix this on MSVC @clemenswasser The bug is there for years, I'm surprised it hasn't been fixed yet. MSVC doesn't support different types with bitf

[PATCH] D139167: [clang][Windows]Ignore Options '/d1nodatetime' and '/d1import_no_registry'

aganea added a comment. In D139167#3967123 , @efriedma wrote: > FYI, there's some discussion of /d1nodatetime floating around, even though it > isn't formally documented: it disables the definition of the `__DATE__`, > `__TIME__`, and `__TIMESTAMP__` ma

[PATCH] D138746: [clang-format] Add .inc extension to git-clang-format

This revision was automatically updated to reflect the committed changes. Closed by commit rGae918c78b51a: [clang-format] Add .inc extension to git-clang-format (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138746/new/ https://

[PATCH] D138746: [clang-format] Add .inc extension to git-clang-format

aganea created this revision. aganea added a reviewer: clang-format. Herald added a project: All. aganea requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The .inc extension isn't picked up for formatting when doing `git clang-format` on the

[PATCH] D137101: [CodeView] Replace GHASH hasher by BLAKE3

This revision was automatically updated to reflect the committed changes. aganea marked 2 inline comments as done. Closed by commit rG49e483d3d62f: [CodeView] Replace GHASH hasher by BLAKE3 (authored by aganea). Herald added a reviewer: MaskRay. Herald added a project: clang. Herald added a subscr

[PATCH] D136474: [CodeView][clang] Disable emitting command line into CodeView by default and add flag to enable

aganea added a comment. In D136474#3888330 , @thakis wrote: >> I read that, and I'm indicating that I don't agree. Chromium's requirements >> are driven by its particular usage of a cross-compiling distributed build >> system, which may not represent th

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

aganea added a comment. In D126903#3885114 , @gchatelet wrote: > It should be fixed now @aganea . Can you check? Works now, thanks again for the quick response. Should D136752 be cherry-picked into the 15.0 branch? In

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

aganea added a comment. Hello @gchatelet! This patches introduces a regression in a two-stage build using rpmalloc on Windows. Bisection lead me here. Would you have a chance to take a look please? Thanks in advance. To reproduce, I used the following script (rename to make_llvm.bat) F25053901:

[PATCH] D136474: [CodeView][clang] Disable emitting command line into CodeView by default and add flag to enable

aganea added subscribers: thieta, saudi. aganea added a comment. +@saudi @thieta Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136474/new/ https://reviews.llvm.org/D136474 ___ cfe-commits mailing list cf

[PATCH] D136474: [CodeView][clang] Add flag to disable emitting command line into CodeView

aganea added a subscriber: stefan_reinalter. aganea added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4354 + +// Emit codeview command line if requested. +if (Args.hasFlag(options::OPT_gcodeview_command_line, aeubanks wrote: > MaskR

[PATCH] D136474: [CodeView][clang] Add flag to disable emitting command line into CodeView

aganea added a comment. Thanks for the patch @aeubanks and apologies for this oversight. I am wondering however if it wouldn’t make more sense to just strip this flag (-fmessage-length) from the emitted cmd-line? The goal is to provide a reproducer, this flag does not matter for that purpose.

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0880b9d52620: [Clang][unittests] Silence trucation warning with MSVC 2022 (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

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

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

aganea updated this revision to Diff 442366. aganea added a comment. Use `#pragma` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129152/new/ https://reviews.llvm.org/D129152 Files: clang/unittests/StaticAnalyzer/RangeSetTest.cpp Index: clang/u

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

aganea created this revision. aganea added reviewers: ASDenysPetrov, thieta, hans. Herald added subscribers: steakhal, mgorny. Herald added a project: All. aganea requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I'm seeing the following with

[PATCH] D86351: WIP: llvm-buildozer

aganea added inline comments. Comment at: llvm/lib/Analysis/MemorySSA.cpp:89 #ifdef EXPENSIVE_CHECKS -bool llvm::VerifyMemorySSA = true; +LLVM_THREAD_LOCAL bool llvm::VerifyMemorySSA = true; #else arsenm wrote: > Do command line flags like this really need to b

[PATCH] D119257: Revert "Re-land [LLD] Remove global state in lldCommon"

aganea added a comment. @mehdi_amini Just to unblock users, I've posted https://reviews.llvm.org/D119277 which fixes the issue originally raised by @krzysz00 I'm also working on a more long term solution in https://reviews.llvm.org/D119049 Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D119257: Revert "Re-land [LLD] Remove global state in lldCommon"

aganea added a comment. All things considered I think in the short term it is better to aim for small patches/fixes that can be transplanted to `release/14.x` if need be. I've posted D119277 for the issue you were seeing @krzysz00, but I am still planning on p

[PATCH] D119257: Revert "Re-land [LLD] Remove global state in lldCommon"

aganea added a comment. @krzysz00 I will revert and re-land with both fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119257/new/ https://reviews.llvm.org/D119257 ___ cfe-commits mailing list cfe-co

[PATCH] D118070: Make lld-link work in a non-MSVC shell

aganea added a comment. In D118070#3289227 , @MaskRay wrote: > I know that you want a place to be accessed by both clang driver and > lld-link but I am a bit nervous about the clang-driver style MSVC library > sitting inside llvm/lib/Support/. > Is the

[PATCH] D118428: [clang-cl] Support the /JMC flag

aganea added inline comments. Comment at: clang/docs/ReleaseNotes.rst:155 +- Add support for MSVC-compatible ``/JMC``/``/JMC-`` flag in clang-cl, and + equivalent -cc1 flag ``-fjmc``. ``/JMC`` could only be used when ``/Zi`` or + ``/Z7`` is turned on. With this addition, clang

[PATCH] D118428: [clang-cl] Support the /JMC flag

aganea added a reviewer: mstorsjo. aganea added a subscriber: belkiss. aganea added a comment. Cool! :) Seems good generally. Sounds like an expensive flag for the runtime perf :-D But I guess it makes the debugging experience a bit nicer. Comment at: clang/lib/Driver/ToolChai

[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. aganea marked 3 inline comments as done. Closed by commit rG5af2433e1794: [clang-cl] Support the /HOTPATCH flag (authored by aganea). Changed prior to commit: https:

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaba5b91b699c: Re-land [CodeView] Add full repro to LF_BUILDINFO record (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D116861: [UBSan] Fix incorrect alignment reported when global new returns an offset pointer

aganea added subscribers: hans, rnk, aganea. aganea added reviewers: hans, rnk. aganea added a comment. +@hans @rnk Since we discussed this in one of the past Windows/COFF calls. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116861/new/ https://rev

[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

aganea marked 2 inline comments as done. aganea added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp:13 +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH +// ERR-HOTPATCH: error: unsupported option '/hotpatch' for t

[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

aganea added inline comments. Comment at: clang/include/clang/Driver/Options.td:2493 MarshallingInfoInt>; +def fhotpatch : Flag<["-"], "fhotpatch">, Group, Flags<[CC1Option]>, + HelpText<"Ensure that all functions can be hotpatched at runtime">, hans wrote: >

[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

aganea updated this revision to Diff 399168. aganea marked 4 inline comments as done. aganea edited the summary of this revision. aganea added a comment. Herald added a subscriber: kristof.beyls. As suggested by @hans Also added test coverage for ARM/ARM64. Repository: rG LLVM Github Monorepo

[PATCH] D116266: [SPIR-V] Add linking of separate translation units using spirv-link

aganea added inline comments. Comment at: clang/test/Driver/spirv-toolchain.cl:71 +// SPLINK: {{llvm-spirv.*"}} [[BC]] "-o" [[SPV2:".*o"]] +// SPLINK: {{"spirv-link.*"}} [[SPV1]] [[SPV2]] "-o" "a.out" svenvh wrote: > aganea wrote: > > Hello @Anastasia, this line

[PATCH] D116266: [SPIR-V] Add linking of separate translation units using spirv-link

aganea added inline comments. Comment at: clang/test/Driver/spirv-toolchain.cl:71 +// SPLINK: {{llvm-spirv.*"}} [[BC]] "-o" [[SPV2:".*o"]] +// SPLINK: {{"spirv-link.*"}} [[SPV1]] [[SPV2]] "-o" "a.out" Hello @Anastasia, this line fails on my machine. It works with

[PATCH] D115103: Leak Sanitizer port to Windows

aganea added a comment. It does work indeed. I am running under a "x64 Native Tools Command Prompt for VS 2019" prompt with MSVC 16.11.8. D:\git\llvm-project>mkdir release && cd release D:\git\llvm-project\release>cmake ../llvm -G Ninja -DLLVM_ENABLE_PROJECTS="clang;lld;llvm;compiler-rt" -D

[PATCH] D116370: [clang][dataflow] Add multi-variable constant propagation example.

aganea added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp:104 + } +} + Hello! I commit a tiny fix here, please see: rGa5af260d3e8b00a3353e813b73f83391edbef493 Repository: rG LLVM Github Monorepo C

[PATCH] D116313: [MSVC] Silence -Wnon-virtual-dtor on DIA APIs

aganea added a subscriber: dblaikie. aganea added a comment. I was wondering, can we mandate `LLVM_ENABLE_WERROR=ON` for all bots at least? (or maybe enable it through cmake, if ever there's a setting to detect we're running as a bot) Or just reverse the default setting on the main branch, ie. m

[PATCH] D116313: [MSVC] Silence -Wnon-virtual-dtor on DIA APIs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe32936aef4a2: [MSVC] Silence -Wnon-virtual-dtor on DIA APIs (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D116313: [MSVC] Silence -Wnon-virtual-dtor on DIA APIs

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

[PATCH] D116313: [MSVC] Silence -Wnon-virtual-dtor on DIA APIs

aganea added inline comments. Comment at: llvm/include/llvm/DebugInfo/PDB/DIA/DIASupport.h:34 +#endif #include +#ifdef __clang__ hans wrote: > I thought this warning should be suppressed for system headers. Isn't that > working here? Good point, because it was

[PATCH] D116313: [MSVC] Silence -Wnon-virtual-dtor on DIA APIs

aganea updated this revision to Diff 397069. aganea marked 2 inline comments as done. aganea added a comment. Herald added subscribers: hiraditya, mgorny. As suggested by @hans Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116313/new/ https://revie

[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

aganea updated this revision to Diff 397063. aganea added a subscriber: saudi. aganea added a comment. Fix tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116511/new/ https://reviews.llvm.org/D116511 Files: clang/include/clang/Basic/CodeGen

[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

aganea created this revision. aganea added reviewers: amccarth, craig.topper, hans, rnk, stefan_reinalter. Herald added subscribers: ormris, dexonsmith, dang, pengfei, hiraditya. aganea requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cf

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

aganea added a comment. @dexonsmith The issue with `-grecord-command-line`/`-frecord-command-line` is that it isn't producing a self-standing command-line. For example: > clang-cl /c main.cpp /clang:-grecord-command-line /Z7 would record: `d:\\git\\llvm-project\\stage1\\bin\\clang-cl.exe --d

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

aganea updated this revision to Diff 396804. aganea added a comment. This revision is now accepted and ready to land. Herald added subscribers: abrachet, mgorny. Rebase & simplify. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/ https://re

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

aganea added inline comments. Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:2470 +// Dynamic TLS initialization works by checking the state of a +// guard varibale (__tls_guard) to see whether TLS initialization +// for a thread has happend yet. s

[PATCH] D116313: [MSVC] Silence -Wnon-virtual-dtor on DIA APIs

aganea created this revision. aganea added reviewers: mstorsjo, amccarth, hans. aganea requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. When building LLVM with Clang 13.0 on Windows, I see a bunch of `-Wnon-virtual-dtor` e

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

aganea added a comment. In D43002#3205190 , @MaskRay wrote: > Some test nits:) I don't know CodeView :( Fixed, thanks @MaskRay ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43002/new/ https://reviews.llv

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/lib/Driver/Job.cpp:390-392 Argv.push_back(nullptr); + Argv.pop_back(); // The terminating null element shall not be part of the + // slice (main() behavior). ---

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/lib/Driver/Job.cpp:390-392 Argv.push_back(nullptr); + Argv.pop_back(); // The terminating null element shall not be part of the + // slice (main() behavior). ---

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

aganea added inline comments. Comment at: clang/lib/Driver/Job.cpp:391 Argv.push_back(nullptr); + Argv.pop_back(); // The terminating null element shall not be part of the + // slice (main() behavior). I changed this to match the behavior of

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd26520f6f787: [Clang] Own the CommandLineArgs in CodeGenOptions (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D116011?vs

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf44e3fbadd15: [CodeView] Emit S_OBJNAME record (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D43002?vs=374884&id=395663#

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

aganea created this revision. aganea added reviewers: hans, aheejin, jansvoboda11. Herald added a subscriber: dexonsmith. aganea requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. As reported in PR52704: https://github.com/ll

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

aganea removed a reviewer: deadalnix. aganea added a comment. Ping! Any further comments? Thanks in advance for your time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43002/new/ https://reviews.llvm.org/D43002 ___

[PATCH] D112498: [Lex] Remove timer from #pragma clang __debug crash handler

aganea added a comment. That sounds right. You can test that scenario in a debugger, by putting a breakpoint before BuryPointer in clang/tools/driver/driver.cpp and verifying that the TimerGroup still has a pointer to the deleted stack frames, after the CrashRecoveryContext has successfully cat

[PATCH] D112498: [Lex] Remove timer from #pragma clang __debug crash handler

aganea added a comment. This Timer is actually the test coverage for that commit. If we don’t want it here, I guess you could move it to the Support unit tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112498/new/ https://reviews.llvm.org/D11

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

aganea added a subscriber: dexonsmith. aganea added a comment. In D80833#3052551 , @dexonsmith wrote: > In D80833#2069411 , @aganea wrote: > >> In D80833#2069246 , @amccarth

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

aganea added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:752 +static void unescapeSlashes(SmallVectorImpl &Str) { + auto Read = Str.begin(); rnk wrote: > aganea wrote: > > rnk wrote: > > > This isn't unescaping them, it's just ca

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

aganea updated this revision to Diff 374884. aganea marked 5 inline comments as done. aganea edited reviewers, added: akhuang; removed: zturner, gratianlup. aganea edited subscribers, added: zturner, gratianlup; removed: aganea. aganea added a comment. Herald added subscribers: dang, mgorny. As su

[PATCH] D109977: LLVM Driver Multicall tool

aganea added a comment. This is really a great change, thanks @beanz! Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:404 -int main(int argc, char **argv) { +int llvm_objcopy_main(int argc, char **argv) { InitLLVM X(argc, argv); Shouldn't we say: ```

[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 +// PERMISSIVE-MINUS-OV

[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 ___ cfe-co

[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 `-fasyn

[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] 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] 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 `main

[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. Howeve

[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: clang/lib/F

[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 Rus

[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 Win

[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 p

[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 of

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

aganea added inline comments. Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:300 + +std::string unescape(StringRef S) { + std::string Out; mstorsjo wrote: > aganea wrote: > > I would also need this function in D43002 (see unescapeSlashes), do you > > think we can m

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

aganea added inline comments. Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:259 -} // anonymous namespace +static bool consume_back_lower(StringRef &S, const char *str) { + if (!S.endswith_lower(str)) `s/str/Str/` Comment at: llvm/tools/llvm-rc

[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] 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 retu

[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] D92191: [clang-scan-deps] Add support for clang-cl

This revision was automatically updated to reflect the committed changes. Closed by commit rGbb26fa8c286b: [clang-scan-deps] Add support for clang-cl (authored by saudi, committed by aganea). Changed prior to commit: https://reviews.llvm.org/D92191?vs=318553&id=338330#toc Repository: rG LLVM

[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&id=338329#toc R

[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: https://reviews.llvm.org/D99973

[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 > nee

[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 wh

[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] 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 https:/

[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: llvm/lib/Support/Windows/Path.inc:1

[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 > htt

[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 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 p

[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 rewrite-inclu

[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 >> https://reviews.llvm.org/D96

[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 https://reviews.llvm.org/

[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$ bar(

[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] 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 one-line

[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 in

  1   2   3   4   5   >