[PATCH] D131730: __has_trivial_copy should map to __is_trivially_copyable

zahen added a comment. In D131730#3718430 , @royjacobson wrote: > Could you add name+email for the commit message? Zachary Henkel za...@microsoft.com Github Username: ZacharyHenkel Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D131730: __has_trivial_copy should map to __is_trivially_copyable

zahen added a comment. In D131730#3718921 , @erichkeane wrote: > Did @royjacobson 's patch make it to the 15.0 branch? If so, Roy: After this > is committed, can you file to get this merged into the 15.x branch? This > seems trivial enough (HAH!) of

[PATCH] D131730: __has_trivial_copy should map to __is_trivially_copyable

zahen created this revision. zahen added reviewers: royjacobson, aaron.ballman, cor3ntin, erichkeane. Herald added a project: All. zahen requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Found during clang 15 RC1 testing due to the new diagno

[PATCH] D101775: Fix for Bug 50033 - -fno-temp-file is not respected when creating a pch in clang 12

zahen added a comment. In D101775#2791938 , @dexonsmith wrote: > @zahen , what author information should I use for the Git commit? Zachary Henkel, za...@microsoft.com (GitHub username ZacharyHenkel) Thanks Duncan! Repository: rG LLVM Github Monorep

[PATCH] D101775: Fix for Bug 50033 - -fno-temp-file is not respected when creating a pch in clang 12

zahen added a comment. @dexonsmith anything else required before this can land? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101775/new/ https://reviews.llvm.org/D101775 ___ cfe-commits mailing list cfe

[PATCH] D101775: Fix for Bug 50033 - -fno-temp-file is not respected when creating a pch in clang 12

zahen added a comment. > If no one has ideas in the next few days, I'll "accept" and commit for you. > (Feel free to ping me; there's a good chance I'll lose track of this > otherwise; usual ping rate is 1x/week, but happy for you to ping me sooner on > this...) @dexonsmith Ping as requested.

[PATCH] D101776: Work around an unfortunate macro in the Windows SDK

zahen added a comment. In D101776#2736003 , @aaron.ballman wrote: >> Once accepted I'll need someone to commit the change on my behalf. > > Thanks for mentioning this up front! What email address and name would you > like to have used for attribution on

[PATCH] D101776: Work around an unfortunate macro in the Windows SDK

zahen updated this revision to Diff 342715. zahen added a comment. Updated the macro based on Aaron's suggestion. clang-formatted the function. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101776/new/ https://reviews.llvm.org/D101776 Files: clang/include/clang/Analysis/CFG.h Inde

[PATCH] D101776: Work around an unfortunate macro in the Windows SDK

zahen created this revision. zahen requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Rename LLVM's single use of CALLBACK as a template parameter to work around the Windows SDK `#define CALLBACK __stdcall` I'm sympathetic to the Michael Bol

[PATCH] D101775: Fix for Bug 50033 - -fno-temp-file is not respected when creating a pch in clang 12

zahen added a comment. Thanks for taking a look right away Duncan! The same concern about a lack of tests was raised when I first added `-fno-temp-file` and I'm afraid inspiration hasn't struck me in the meantime. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D101775: Fix for Bug 50033 - -fno-temp-file is not respected when creating a pch in clang 12

zahen created this revision. zahen added reviewers: dexonsmith, erik.pilkington. zahen requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When creating a PCH file the use of a temp file will be dictated by the presence or absence of the -fno-

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

zahen added a comment. This patch doesn't need a test case outside of the one that @rnk requested to make sure that the flag flows from the clang-cl driver appropriately. `pch-instantiate-templates` as authored doesn't match cl.exe behavior and being unable to turn it off will prevent our adop

[PATCH] D86622: Fix failing tests after VCTOOLSDIR change

zahen added a comment. Sorry again about the break! As with the initial patch, I'll need you to land this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86622/new/ https://reviews.llvm.org/D86622 __

[PATCH] D86622: Fix failing tests after VCTOOLSDIR change

zahen created this revision. zahen added a reviewer: hans. Herald added a project: clang. Herald added a subscriber: cfe-commits. zahen requested review of this revision. Switch from hardcoded x64 arch to a regex in the target triple Repository: rG LLVM Github Monorepo https://reviews.llvm.or

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

zahen added a comment. Thanks @hans! I'll need you (or someone else with permission) to land the change on my behalf. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85998/new/ https://reviews.llvm.org/D85998 ___ cfe-commits mailing list cfe-

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

zahen added a comment. The real reason we don't see it internally is because we use -c for all compilation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85998/new/ https://reviews.llvm.org/D85998 ___ cfe-commits mailing list cfe-commits@lis

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

zahen marked an inline comment as done. zahen added a comment. In D85998#2236401 , @hans wrote: > I'm still curious why it seems it's not looking for link.exe in the /fake dir > though. clang\lib\Driver\ToolChains\MSVC.cpp:311 FindVisualStudioExecutable

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

zahen marked 3 inline comments as done. zahen added inline comments. Comment at: clang/test/Driver/cl-options.c:686 +// 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 fake

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

zahen updated this revision to Diff 287671. zahen added a comment. Herald added a subscriber: ormris. 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 lin

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

zahen added inline comments. Comment at: clang/test/Driver/cl-options.c:686 +// 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 + zahen

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

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

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

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

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

zahen marked an inline comment as done. zahen added a comment. @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. Comment at: clang/l

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

zahen updated this revision to Diff 287443. zahen added a comment. Good call @aganea on these comments. Updated CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85998/new/ https://reviews.llvm.org/D85998 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/MSVC.cpp

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

zahen updated this revision to Diff 287425. zahen edited reviewers, added: aganea; removed: thakis. zahen added a comment. Updates as requested CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85998/new/ https://reviews.llvm.org/D85998 Files: clang/include/clang/Driver/Options.td clan

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

zahen added a comment. We use BuildXL. Thanks for the comments. I'll make the requested changes and get a new rev posted. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85998/new/ https://reviews.llvm.org/D85998 ___ cfe-commits mailing list

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

zahen added a comment. 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 p

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

zahen added a comment. Once accepted, someone else will need to submit on my behalf. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:74 + if (Arg *A = Args.getLastArg(options::OPT__SLASH_vctoolsdir)) { +Path = A->getValue(); +VSLayout = MSVCToolChain::ToolsetLayout::

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

zahen updated this revision to Diff 285803. zahen added a comment. Fix formatting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85998/new/ https://reviews.llvm.org/D85998 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/MSVC.cpp clang/test/Driver/cl-option

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

zahen created this revision. zahen added reviewers: rnk, hans, thakis. Herald added subscribers: cfe-commits, dang. Herald added a project: clang. zahen requested review of this revision. Add an option to directly specify where the msvc toolchain lives for clang-cl and avoid unwanted file and reg

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

zahen added a comment. 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? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72405/new

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

zahen updated this revision to Diff 237795. zahen added a comment. Incorporate review feedback. I had no luck converting the CHECK-FOO & CHECK-NOFOO tests to use `-verify` because the errors were reported against "(frontend)" error: 'error' diagnostics seen but not expected: (frontend): d

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

zahen added a comment. 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

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

zahen created this revision. zahen added reviewers: rnk, thakis, hans. Herald added a project: clang. Herald added a subscriber: cfe-commits. Before this patch adding a new /D flag when compiling a source file that consumed a PCH with clang-cl would issue a diagnostic and then fail. With the pa

[PATCH] D70615: Add an -fno-temp-file flag for compilation

zahen updated this revision to Diff 234563. zahen added a comment. Fixed MemorySanitizer: use-of-uninitialized-value warning CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70615/new/ https://reviews.llvm.org/D70615 Files: clang/include/clang/Driver/Options.td clang/include/clang/Fro

[PATCH] D70615: Add an -fno-temp-file flag for compilation

zahen reopened this revision. zahen added a comment. This revision is now accepted and ready to land. Sorry about that I'll add a corrected patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70615/new/ https://reviews.llvm.org/D70615 __

[PATCH] D71439: Allow redeclaration of __declspec(uuid)

zahen added a comment. @rnk Can you please submit on my behalf. I don't have commit access. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71439/new/ https://reviews.llvm.org/D71439 ___ cfe-commits mai

[PATCH] D70615: Add an -fno-temp-file flag for compilation

zahen added a comment. Any additional changes required? If not could someone please submit on my behalf? @rnk, @hans, @thakis ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70615/new/ https://reviews.llvm.org/D70615 ___ cfe-commits mail

[PATCH] D71439: Allow redeclaration of __declspec(uuid)

zahen created this revision. zahen added reviewers: rnk, thakis, hans, zturner. Herald added a project: clang. Herald added a subscriber: cfe-commits. msvc allows a subsequent declaration of a uuid attribute on a struct/class. Mirror this behavior in clang-cl. Repository: rG LLVM Github Mono

[PATCH] D70615: Add an -fno-temp-file flag for compilation

zahen added a comment. Thanks Hans! I'll need someone to do the actual submission since I don't have commit rights. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70615/new/ https://reviews.llvm.org/D70615 ___ cfe-commits mailing list cfe-

[PATCH] D70615: Add an -fno-temp-file flag for compilation

zahen added a comment. 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. I've uploaded example ProcMon dumps of clang-cl.exe file accesses t

[PATCH] D70615: Add an -fno-temp-file flag for compilation

zahen added a comment. In D70615#1770550 , @hans wrote: > 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. Added a best

[PATCH] D70615: Add an -fno-temp-file flag for compilation

zahen updated this revision to Diff 232442. zahen added a comment. Fixed patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70615/new/ https://reviews.llvm.org/D70615 Files: clang/include/clang/Driver/Options.td clang/include/clang/Frontend/FrontendOptions.h clang/lib/Driver/Too

[PATCH] D70615: Add an -fno-temp-file flag for compilation

zahen updated this revision to Diff 232441. zahen added a comment. Updated with review feedback and formatting fixes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70615/new/ https://reviews.llvm.org/D70615 Files: clang/include/clang/Driver/Options.td clang/lib/Frontend/CompilerInst

[PATCH] D70615: Add an -fno-temp-file flag for compilation

zahen created this revision. zahen added reviewers: rsmith, rnk, zturner. Herald added a project: clang. Herald added a subscriber: cfe-commits. Our build system does not handle randomly named files created during the build well. We'd prefer to write compilation output directly without creating

[PATCH] D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode.

zahen added a comment. 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-spe

[PATCH] D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode.

zahen added a comment. 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

[PATCH] D62752: Move VarBypassDector.h to include/clang/CodeGen

zahen created this revision. zahen added a reviewer: chandlerc. Herald added a project: clang. Herald added a subscriber: cfe-commits. Allow the VarBypassDetector class to be consumed by external tools by exposing its header from a standard include path. If accepted, I will need a user with chec

[PATCH] D57189: Fix compatibility with the msvc AI compiler option

zahen created this revision. zahen added reviewers: rnk, zturner. Herald added a subscriber: cfe-commits. The unsupported /AI option accepts directory paths in a manner similar to the supported /I option and allows spaces between the flag and the directory. An example snippet from our productio

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

zahen added a comment. I don't have check-in permission, so I'd appreciate if someone could handle the actual commit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55685/new/ https://reviews.llvm.org/D55685 ___ cfe-commits mailing list cfe

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

zahen updated this revision to Diff 178490. zahen added a comment. Added support for msvc minor version as requested. Tied the updated mangling scheme to C++17 & compatibility mode > 1912 (15.5). Added additional tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55685/new/ https:/

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

zahen added a comment. In D55685#1331810 , @zturner wrote: > 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

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

zahen added a comment. In D55685#1330717 , @zturner wrote: > 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`) This was

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

zahen created this revision. zahen added reviewers: zturner, rnk. Herald added a subscriber: cfe-commits. The msvc exception specifier for noexcept function types has changed from the prior default of "Z" to "_E" if the function cannot throw when compiling with /std:C++17. Repository: rC Cla