Page MenuHomePhabricator

zahen (Zachary Henkel)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 13 2018, 6:10 PM (92 w, 2 d)

Recent Activity

Wed, Sep 2

zahen added a reviewer for D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver: rnk.
Wed, Sep 2, 7:07 AM · Restricted Project

Wed, Aug 26

zahen added a comment to rG33ce275fc156: [Clang] Fix tests following rG087047144210.

Thanks!

Wed, Aug 26, 8:40 AM
zahen added a comment to D86622: Fix failing tests after VCTOOLSDIR change.

Sorry again about the break! As with the initial patch, I'll need you to land this change.

Wed, Aug 26, 7:33 AM · Restricted Project
zahen requested review of D86622: Fix failing tests after VCTOOLSDIR change.
Wed, Aug 26, 7:28 AM · Restricted Project

Tue, Aug 25

zahen added a comment to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

Thanks @hans! I'll need you (or someone else with permission) to land the change on my behalf.

Tue, Aug 25, 10:51 AM · Restricted Project
zahen added a comment to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

The real reason we don't see it internally is because we use -c for all compilation.

Tue, Aug 25, 8:59 AM · Restricted Project
zahen added a comment to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

I'm still curious why it seems it's not looking for link.exe in the /fake dir though.

Tue, Aug 25, 8:48 AM · Restricted Project
zahen added inline comments to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.
Tue, Aug 25, 8:14 AM · Restricted Project
zahen updated the diff for D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

It felt too invasive to swap the precedence of using vctoolsdir vs %INCLUDE% for all clang-cl users so I compromised by skipping the %INCLUDE% check when vctoolsdir was passed on the command line. This way only the users of the new flag will see different behavior.

Tue, Aug 25, 8:12 AM · Restricted Project

Mon, Aug 24

zahen added inline comments to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.
Mon, Aug 24, 1:00 PM · Restricted Project
zahen added inline comments to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.
Mon, Aug 24, 11:05 AM · Restricted Project
zahen added inline comments to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.
Mon, Aug 24, 11:03 AM · Restricted Project
zahen added a comment to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

@hans the explicit path must be declared, all exes and dlls. The unexpected probes for link.exe when invoking clang-cl.exe when it isn't actually going to be invoked are what we want to avoid.

Mon, Aug 24, 10:45 AM · Restricted Project
zahen updated the diff for D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

Good call @aganea on these comments. Updated

Mon, Aug 24, 10:34 AM · Restricted Project
zahen updated the diff for D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

Updates as requested

Mon, Aug 24, 9:48 AM · Restricted Project

Fri, Aug 21

zahen added a comment to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

We use BuildXL. Thanks for the comments. I'll make the requested changes and get a new rev posted.

Fri, Aug 21, 2:56 PM · Restricted Project
zahen added a comment to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

The build system strives to be deterministic so all file probes need to be accounted for; environment variables are also problematic. Our builds deliberately don't run from a Visual Studio command prompt. In addition, we'd like to avoid coupling clang tools that don't perform codegen to link.exe. We do use -fmsc-version= during the build.

Fri, Aug 21, 12:23 PM · Restricted Project

Aug 15 2020

zahen added a comment to D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

Once accepted, someone else will need to submit on my behalf.

Aug 15 2020, 2:34 PM · Restricted Project

Aug 14 2020

zahen updated the diff for D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.

Fix formatting

Aug 14 2020, 5:54 PM · Restricted Project
zahen requested review of D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain.
Aug 14 2020, 2:15 PM · Restricted Project

Jan 14 2020

zahen added a comment to D72405: Allow /D flags absent during PCH creation under msvc-compat.

Wild guess is that 2> %t.err should be removed from the -verify lines? That change passes in the single env I have access to. @rnk any idea what might be going on?

Jan 14 2020, 7:42 PM · Restricted Project

Jan 13 2020

zahen updated the diff for D72405: Allow /D flags absent during PCH creation under msvc-compat.

Incorporate review feedback.

Jan 13 2020, 3:26 PM · Restricted Project
zahen added a reviewer for D72405: Allow /D flags absent during PCH creation under msvc-compat: zturner.
Jan 13 2020, 7:35 AM · Restricted Project

Jan 9 2020

zahen added a comment to D72405: Allow /D flags absent during PCH creation under msvc-compat.

My change keeps the diagnostic so consumers can opt into the same enforcement that exists today. Furthermore, the existing fuzzy-pch.c tests show that new -D flags are allowed under a "clangier" PCH structure. None of the existing tests error on:

BAR bar = 17;

when -DBar=int is only part of the the test file's command line.

Jan 9 2020, 8:11 PM · Restricted Project

Jan 8 2020

zahen created D72405: Allow /D flags absent during PCH creation under msvc-compat.
Jan 8 2020, 11:18 AM · Restricted Project

Dec 18 2019

zahen updated the diff for D70615: Add an -fno-temp-file flag for compilation.

Fixed MemorySanitizer: use-of-uninitialized-value warning

Dec 18 2019, 9:41 AM · Restricted Project
zahen reopened D70615: Add an -fno-temp-file flag for compilation.

Sorry about that I'll add a corrected patch

Dec 18 2019, 9:36 AM · Restricted Project

Dec 17 2019

zahen added a comment to D70615: Add an -fno-temp-file flag for compilation.

Any additional changes required? If not could someone please submit on my behalf? @rnk, @hans, @thakis ?

Dec 17 2019, 8:10 PM · Restricted Project
zahen added a comment to D71439: Allow redeclaration of __declspec(uuid).

@rnk Can you please submit on my behalf. I don't have commit access.

Dec 17 2019, 8:10 PM · Restricted Project

Dec 12 2019

zahen created D71439: Allow redeclaration of __declspec(uuid).
Dec 12 2019, 3:42 PM · Restricted Project

Dec 10 2019

zahen added a comment to D70615: Add an -fno-temp-file flag for compilation.

Thanks Hans! I'll need someone to do the actual submission since I don't have commit rights.

Dec 10 2019, 6:47 AM · Restricted Project

Dec 8 2019

zahen added a comment to D70615: Add an -fno-temp-file flag for compilation.

In our build system every file access in an "output directory" needs to be accounted for. Until this patch, the random temporary file name has forced us to rely on workarounds that hurt build throughput.

Dec 8 2019, 8:37 PM · Restricted Project

Dec 5 2019

zahen added a comment to D70615: Add an -fno-temp-file flag for compilation.

Seems fine to me. I'm not sure how to test the actual "don't write to temp file" functionality, but at least there could be a test to check that the flag gets forwarded to cc1.

Dec 5 2019, 1:59 PM · Restricted Project
zahen updated the diff for D70615: Add an -fno-temp-file flag for compilation.

Fixed patch

Dec 5 2019, 1:31 PM · Restricted Project
zahen updated the diff for D70615: Add an -fno-temp-file flag for compilation.

Updated with review feedback and formatting fixes

Dec 5 2019, 1:25 PM · Restricted Project

Nov 22 2019

zahen created D70615: Add an -fno-temp-file flag for compilation.
Nov 22 2019, 1:11 PM · Restricted Project

Sep 17 2019

zahen added a comment to D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode..

I just checked a trivial mismatch example in clang 8.0 (https://godbolt.org/z/wiSAp6) and I missed how the compatibility mode operates. I withdraw my suggestion since the patch keeps consistency with the existing behavior. Thanks for the -Werror=microsoft-exception-spec suggestion that's exactly what our project wants.

Sep 17 2019, 8:42 AM · Restricted Project, Restricted Project
zahen added a comment to D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode..

Is there any interest in supporting the cl.exe flag /permissive-? I considered a hard error on mismatched exception specifier in clang-cl a feature, not a bug. If msvc compat mode respected that flag this could continue to be an error with that flag set (and upgraded strictness in other cases).

Sep 17 2019, 8:05 AM · Restricted Project, Restricted Project

Sep 12 2019

zahen abandoned D62752: Move VarBypassDector.h to include/clang/CodeGen.
Sep 12 2019, 9:14 AM · Restricted Project

May 31 2019

zahen created D62752: Move VarBypassDector.h to include/clang/CodeGen.
May 31 2019, 2:52 PM · Restricted Project

Jan 24 2019

zahen created D57189: Fix compatibility with the msvc AI compiler option.
Jan 24 2019, 1:46 PM

Jan 8 2019

zahen added a comment to D55769: Add support for demangling msvc's noexcept types to llvm-undname.

lgtm. I'm technically on vacation so rnk@ might be able to commit it sooner than I can, but if he doesn't, then I'll commit it when I find some spare cycles. Thanks for fixing this!

Jan 8 2019, 11:26 AM

Dec 17 2018

zahen added a comment to D55769: Add support for demangling msvc's noexcept types to llvm-undname.

Same as the corresponding mangling patch, I'd appreciate someone else landing this since I don't have check-in permission.

Dec 17 2018, 3:19 PM
zahen added a comment to D55685: Update Microsoft name mangling scheme for exception specifiers in the type system.

I don't have check-in permission, so I'd appreciate if someone could handle the actual commit.

Dec 17 2018, 1:05 PM
zahen updated the diff for D55685: Update Microsoft name mangling scheme for exception specifiers in the type system.

Added support for msvc minor version as requested. Tied the updated mangling scheme to C++17 & compatibility mode > 1912 (15.5). Added additional tests.

Dec 17 2018, 10:21 AM
zahen created D55769: Add support for demangling msvc's noexcept types to llvm-undname.
Dec 17 2018, 8:07 AM

Dec 14 2018

zahen added a comment to D55685: Update Microsoft name mangling scheme for exception specifiers in the type system.

BTW, as far as updating the demangler, as long as it doesn't crash or generate an error on a valid _E mangling, that should be sufficient (with a test). If you want bonus points you can make it print out noexcept when you see the _E, but I won't require it as it's a bit of extra work. If you decide not to do that, I'll just file a bug for it so that we don't forget.

Dec 14 2018, 3:16 PM
zahen added a comment to D55685: Update Microsoft name mangling scheme for exception specifiers in the type system.

Also we still need to put this behind -fms-compatibility-version. Finally, it would be nice if you could also update the demangler (llvm/lib/Demangle/MicrosoftDemangle.cpp)

Dec 14 2018, 2:46 PM

Dec 13 2018

zahen created D55685: Update Microsoft name mangling scheme for exception specifiers in the type system.
Dec 13 2018, 6:19 PM