[PATCH] D61418: Another attempt to fix "could not find clang-check" lit warning in analyzer-less builds

thakis created this revision. thakis added a reviewer: rnk. Herald added subscribers: dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware. r359717 added clang-check as a dep of check-clang unconditionally because I had missed lit.local.cfg in test/Tooling. Instead, only add clang-check

[PATCH] D61456: [gn] Update the clangd test lit site configuration

thakis added a comment. Thanks! This is incomplete: - Create clangd_lit_site_cfg_files.gni next to this build file, but path to generated files there - Edit llvm/utils/gn/secondary/llvm/utils/llvm-lit/BUILD.gn and add remapping there Else `out/gn/bin/llvm-lit clang-tools-extra/clangd/test/path

[PATCH] D61456: [gn] Update the clangd test lit site configuration

thakis closed this revision. thakis added a comment. This landed in r359825. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61456/new/ https://reviews.llvm.org/D61456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D48292: use modern type trait implementations when available

thakis added a comment. This test is broken with Xcode 9: -- Testing: 53613 tests, 8 threads -- Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90 FAIL: libc++ :: std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp (50845 of 53613)

[PATCH] D48292: use modern type trait implementations when available

thakis added a comment. Done in r359907, thanks! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48292/new/ https://reviews.llvm.org/D48292 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

thakis added a comment. I happened to see this go by. Is there an explanation of the overall goal somewhere? In general, requiring -- for long flags sounds like a great change to me, but there are a few exceptions: For example. lld-link should keep accepting long flags with a single dash for li

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

thakis added a comment. Thanks for the context. Sounds great :) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61269/new/ https://reviews.llvm.org/D61269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D61646: Include corecrt.h/vcruntime.h to improve MS compatibility

thakis added a comment. Did I get the bug right that this adds almost 400kB to every file that includes stddef.h? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61646/new/ https://reviews.llvm.org/D61646 ___ cfe-commi

[PATCH] D61646: Include corecrt.h/vcruntime.h to improve MS compatibility

thakis added a comment. I agree with that. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61646/new/ https://reviews.llvm.org/D61646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bi

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

thakis added inline comments. Comment at: include/clang/Driver/CLCompatOptions.td:129 +def _SLASH_Oy : CLFlag<"Oy">, Alias<_SLASH_O>, AliasArgs<["y"]>, HelpText<"Enable frame pointer omission (x86 only)">; +def _SLASH_Oy_ : CLFlag<"Oy-">, Alias<_SLASH_O>, AliasArgs<["y-"]>, Hel

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

thakis added a comment. FWIW the recommendation against /Ox in my version is because of https://github.com/ulfjack/ryu/pull/70#issuecomment-412168459 https://reviews.llvm.org/D52266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

thakis added a comment. Since it helps existing msbuild configs, adding this seems like a good thing to me. clang-cl isn't supposed to do (explicit) registry accesses when you hold it right (pass in -fms-compatibility-version etc). Have you seen registry access costs, or is that speculation?

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

thakis added a comment. In https://reviews.llvm.org/D52266#1240304, @hans wrote: > Sorry, I didn't realize we both set off to do this in parallel. I've > incorporated your changes into my patch. No worries, I didn't do anything I wouldn't have done for reviewing this :-) Thoughts on "As far a

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

thakis added inline comments. Comment at: include/clang/Driver/CLCompatOptions.td:121 +def : CLFlag<"Ob0">, Alias<_SLASH_O>, AliasArgs<["b0"]>, HelpText<"Disable function inlining">; +def : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>, HelpText<"Only inline functions which

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

thakis added a comment. In https://reviews.llvm.org/D52193#1241067, @aganea wrote: > @thakis > clang-cl isn't supposed to do (explicit) registry accesses when you > hold it right (pass in -fms-compatibility-version etc). Have you seen > registry access costs, or is that speculation? > > Please

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

thakis added a comment. ...and to reword this a bit: Clang taking a long time to start up in some configurations is a bug we should profile and fix :-) Repository: rC Clang https://reviews.llvm.org/D52193 ___ cfe-commits mailing list cfe-commits

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. lgtm, thanks! Maybe file a bug on figuring out the /Gs story and add a FIXME linking to it. Weird. Comment at: include/clang/Driver/CLCompatOptions.td:118 +def _SLASH_O : C

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

thakis added a comment. > Actually, trying this out with MSVC, I don't see any __chkstk calls with > /https://reviews.llvm.org/owners/package/1/, or with eg. /Gs1 for that matter: (.guess the bug should also cover to eventually not alias /Gs to /Gs0 and/or /Gs to -mstack-probe-size either if cl

[PATCH] D52499: [clang-cl] Make /Gs imply default stack probes, not /Gs0 (PR39074)

thakis added a comment. Thanks! lgtm. Comment at: include/clang/Driver/CLCompatOptions.td:94 +def : CLFlag<"Gs">, HelpText<"Use stack probes (default)">, + Alias, AliasArgs<["4096"]>; def _SLASH_Gs : CLJoined<"Gs">, https://docs.microsoft.com/en-us/cpp/build/

[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Looks good. If this is passed and we invoke lld-link, should we give that a similar flag and pass that to lld-link as well? I think link.exe also prints its outputs. Commen

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

thakis created this revision. thakis added a reviewer: djasper. The existing code kept the space if it was there for identifiers, and it didn't handle `this`. After this patch, for java `this` is handled in addition to identifiers, and existing space is always stripped between identifier and `::

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

thakis updated this revision to Diff 168161. thakis edited the summary of this revision. https://reviews.llvm.org/D52842 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestJava.cpp Index: lib/Format/TokenAnnotator.cpp

[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)

thakis added a comment. In https://reviews.llvm.org/D52773#1255093, @zturner wrote: > I agree magic environment variables are bad, but without it we don’t > address the only current actual use we have for this, which is making the > vs integration print filenames. Detecting compiler version fro

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

thakis added a comment. krasimir, do you do clang-format reviews these days? If not, do you know who does? https://reviews.llvm.org/D52842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

thakis added a comment. Thanks! Will land with tweaked comment. Comment at: lib/Format/TokenAnnotator.cpp:2559 // A.>>DoSomething(); +// A::>>DoSomething(); return false; krasimir wrote: > nit: please add a comment that this example comes from Jav

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

thakis added a comment. It looks like you ran clang-format on all of lib/Sema/SemaExpr.cpp and changed many lines that are irrelevant to your patch. Can you undo that, please? https://reviews.llvm.org/D52949 ___ cfe-commits mailing list cfe-commits

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

thakis added a comment. Looking at D54630 , it seems arphaman and dexonsmith are probalby good reviewers for this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57532/new/ https://reviews.llvm.org/D57532 ___

[PATCH] D57592: Replace uses of %T with %t in from previous frontend test differential

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Normally when needing a directory you'd `mkdir %t.dir` before using it, but since you don't' want the directory to exist here this should be good as-is. Thanks! Repository: rC Clang CHANG

[PATCH] D16951: [MS ABI] dllimport'd class cannot have constexpr ctors

thakis closed this revision. thakis added a comment. David landed a better fix in r260388. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D16951/new/ https://reviews.llvm.org/D16951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D57592: Replace uses of %T with %t in from previous frontend test differential

thakis added a comment. Done, thanks for the patch! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57592/new/ https://reviews.llvm.org/D57592 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D54978: Move the SMT API to LLVM

thakis added a comment. In D54978#1389260 , @shakeel.mahate wrote: > The ninja build also fails That's not the ninja build, that's the GN build. If you use that, please read llvm/utils/gn/README.rst. If something like this happens, it's on you (and ot

[PATCH] D54978: Move the SMT API to LLVM

thakis added a comment. In D54978#1389110 , @brzycki wrote: > This commit is causing a build-break for our nightly cross compilers of arm > and aarch64. The immediately preceding commit of D54977 > does not break with th

[PATCH] D57127: [analyzer] Port RetainSummaryManager to the new GenericCall interface, decouple ARCMT from the analyzer

thakis added a comment. Herald added a project: clang. Did you see the "Should we remove this check that currently requires ENABLE_ARCMT => ENABLE_SA? http://llvm-cs.pcc.me.uk/tools/clang/CMakeLists.txt#441"; question? In D57127#1373949 , @george.karpenk

[PATCH] D54978: Move the SMT API to LLVM

thakis added a comment. Thanks for the analysis. I think it's fine if you revert, given that. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54978/new/ https://reviews.llvm.org/D54978 ___ cfe-commits mailing list cfe-

[PATCH] D54978: Move the SMT API to LLVM

thakis added a comment. In D54978#1391436 , @mikhail.ramalho wrote: > Hi everyone, I just saw your messages and reverted the commits. > > Sorry for the inconvenience, but for some reason I didn't get any email from > the bots. Could you send me the link

[PATCH] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

thakis added a comment. I just ran into this and I'm a bit confused about the discussion here. This snippet (in a .c file) #include static_assert(4 == 4 , ""); builds in all compilers except clang-cl. How does not supporting this make sense? Instead of this patch we could have an asse

[PATCH] D38606: [CodeGen] Emit a helper function for __builtin_os_log_format to reduce code size

thakis added inline comments. Herald added subscribers: llvm-commits, jkorous. Herald added a project: LLVM. Comment at: cfe/trunk/test/CodeGenObjC/os_log.m:53 + // CHECK-O0: %[[V5:.*]] = bitcast %[[V0]]* %[[V3]] to i8* + // CHECK-O0-NOT call void (...) @clang.arc.use({{.*}} +

[PATCH] D58061: Fix a few tests that were missing ':' on CHECK lines and weren't testing anything.

thakis created this revision. thakis added a reviewer: rjmccall. Herald added subscribers: kristof.beyls, javed.absar. Found by `git grep '\/\/ CHECK-[^: ]* ' clang/test/ | grep -v RUN:`. Also tweak CodeGenCXX/arm-swiftcall.cpp to still pass now that it checks more. https://reviews.llvm.org/D58

[PATCH] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

thakis added a comment. In D17444#1393339 , @rnk wrote: > In D17444#1392823 , @thakis wrote: > > > Instead of this patch we could have an assert.h wrapper in lib/Headers that > > defines static_assert to _Static_as

[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

thakis added a comment. In D57915#1389722 , @TomTan wrote: > In D57915#1389560 , @lebedev.ri > wrote: > > > In D57915#1389549 , @TomTan wrote: > > > > > Added the tests back

[PATCH] D54978: Move the SMT API to LLVM

thakis added a comment. > In D54978#1393552 , @ddcc wrote: > >> The likely reason for this versioning problem is that the current versioning >> implementation in FindZ3.cmake is best-effort only. > > > Thank you @ddcc for this explanation. If that's the

[PATCH] D54978: Move the SMT API to LLVM

thakis added a comment. Herald added a subscriber: jdoerfert. >> If so, I don't understand why the default setting is important to you and >> why this doesn't work for you. (I don't disagree with the default being off, >> I'm just confused why things don't work for you.) > > As I have stated se

[PATCH] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

thakis added a comment. Herald added a subscriber: jdoerfert. I don't have an opinion if this patch here or the assert wrapper is better, but I feel somewhat strongly that we should land one or the other so that the standard C program #include static_assert(4 == 4 , ""); builds in clan

[PATCH] D58161: Print a note to the called macro when diagnosing err_embedded_directive

thakis created this revision. thakis added a reviewer: rnk. Fixes PR40713, see there for the motivation for this. https://reviews.llvm.org/D58161 Files: clang/include/clang/Basic/DiagnosticLexKinds.td clang/include/clang/Lex/Preprocessor.h clang/lib/Lex/PPDirectives.cpp clang/lib/Lex/PP

[PATCH] D54978: Move the SMT API to LLVM

thakis added a comment. In D54978#1395136 , @brzycki wrote: > The following patch: > > diff --git a/llvm/cmake/modules/CrossCompile.cmake > b/llvm/cmake/modules/CrossCompile.cmake > index bc3b210f018..0c30b88f80f 100644 > --- a/llvm/cmake/modules/Cr

[PATCH] D58161: Print a note to the called macro when diagnosing err_embedded_directive

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:497 + Args = ReadMacroCallArgumentList(Identifier, MI, ExpansionEnd); rnk wrote: > Can this re-enter, something like: > MACRO1(MACRO2(a1

[PATCH] D58157: Stop enabling clang-tools-extra automatically when clang is in LLVM_ENABLE_PROJECTS

thakis added a comment. In D58157#1395762 , @mehdi_amini wrote: > In D58157#1395716 , @rnk wrote: > > > I think we have consensus, > > > Based on three comments in a revision? Seems strange to me. > I don't really

[PATCH] D45978: dllexport const variables must have external linkage.

thakis added inline comments. Comment at: lib/Sema/SemaDecl.cpp:11370 +// In Microsoft C++ mode, a const variable defined in namespace scope has +// external linkage by default if the variable is declared with Even in unnamed namespaces? CHANGES SINCE

[PATCH] D58161: Print a note to the called macro when diagnosing err_embedded_directive

thakis marked 2 inline comments as done. thakis added a comment. Thanks! Comment at: clang/lib/Lex/PPMacroExpansion.cpp:497 + Args = ReadMacroCallArgumentList(Identifier, MI, ExpansionEnd); rnk wrote: > thakis wrote: > > rnk wrote: > > > Can this re-ente

[PATCH] D58157: Stop enabling clang-tools-extra automatically when clang is in LLVM_ENABLE_PROJECTS

thakis added a comment. Thanks for starting the thread, Reid. In D58157#1397260 , @mehdi_amini wrote: > Sorry, I don't have time to do archeology for you right now. But this is > beside the point: your patch is changing a 2 years status quo, so my take o

[PATCH] D17444: [MSVC] Recognize "static_assert" keyword in C mode

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Awesome, thanks. Comment at: clang/test/Lexer/keywords_test.cpp:57 +#else +// MS compiler recognizes static_assert in all modes. So should we. +IS_KEYWORD(static_assert); ---

[PATCH] D58377: Remove extraneous space in MSVC-style diagnostic output

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. wht CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58377/new/ https://reviews.llvm.org/D58377 ___ cfe-commits mailing list cfe-commit

[PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

thakis created this revision. Makes the warning useful again in a std::unique_ptr world, PR28460. https://reviews.llvm.org/D37235 Files: include/clang/Basic/DiagnosticSemaKinds.td test/SemaCXX/destructor.cpp Index: test/SemaCXX/destructor.cpp ==

[PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

thakis updated this revision to Diff 112981. thakis edited the summary of this revision. thakis added a comment. Don't warn in unevaluated contexts. Ready for a look now. https://reviews.llvm.org/D37235 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExprCXX.cpp test/SemaCX

[PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

thakis closed this revision. thakis added a comment. Thanks, landed in r312167. dblaikie: I believe the test can't be much simpler -- the pragma is file-based and having the main TU be marked as system header would be fairly different from what happens in real life even if it worked. https://

[PATCH] D37324: Suppress -Wdelete-non-virtual-dtor warnings _about_ classes defined in system headers.

thakis created this revision. r312167 made it so that we emit Wdelete-non-virtual-dtor from `delete` statements that are in system headers (e.g. std::unique_ptr). That works great on Linux and macOS, but on Windows there are non-final classes that are defined in system headers that have virtual

[PATCH] D37324: Suppress -Wdelete-non-virtual-dtor warnings _about_ classes defined in system headers.

thakis closed this revision. thakis added a comment. 312216, thanks! https://reviews.llvm.org/D37324 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37954: Expand absolute system header paths when using -MD depfiles

thakis added a comment. I think for depfile generation, we generally try to be gcc-compatible (I can link to prior changes in this spirit), so I think this seems like a good thing to do to me. gcc only does this for system headers, yes? Does gcc support the non-negated spelling of this flag (-

[PATCH] D62746: msabi: Fix exponential mangling time in certain pathological inputs

thakis created this revision. thakis added a reviewer: rnk. Template back references used to be recursively recomputed, add a memoization cache to cut down on this. Since there are now two different types of argument maps, rename the existing TypeBackReferences to FunArgBackReferences, and rename

[PATCH] D62746: msabi: Fix exponential mangling time for certain pathological inputs

This revision was automatically updated to reflect the committed changes. Closed by commit rG73f05841992c: msabi: Fix exponential mangling time for certain pathological inputs (authored by thakis). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D62746?vs=202472

[PATCH] D62780: msabi: Fix exponential mangling time for even more contrived inputs

thakis created this revision. thakis added a reviewer: rnk. This is a follow-up to r362293 which fixed exponential time needed for mangling certain templates. This fixes the same issue if that template pattern happens in template arguments > 10: The first ten template arguments can use back refere

[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

thakis added a comment. From what I can tell, the only client of OpenCLBuiltins.td is currently lib/Sema. Do you expect that to change? If not, does it make more sense to move the .td file to there? Do you expect OpenCLBuiltins.td to include other files in the future? At the moment, the `-I ${

[PATCH] D62780: msabi: Fix exponential mangling time for even more contrived inputs

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:819-823 ArgBackRefMap::iterator Found = TemplateArgBackReferences.find(ND); if (Found == TemplateArgBackReferences.end()) { - // Mangle full templat

[PATCH] D55463: Introduce a source minimizer that reduces source to directives that might affect the dependency list for a compilation

thakis added a comment. Any reason this isn't tested via lit instead? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55463/new/ https://reviews.llvm.org/D55463 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

thakis added a comment. The test is failing on some bots, e.g. http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/13430/steps/test-stage1-compiler/logs/stdio /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/test/Preprocessor/arm-target-features.c:762:20: e

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

thakis added a comment. I reverted this in r363379 to unbreak check-lld on mac. I think the if config.enable_shared: features.add("enable_shared") bit belongs in clang/test/lit.cfg.py, not in llvm/utils/lit/lit/llvm/config.py. Repository: rL LLVM CHANGES SINCE LAST ACTION

[PATCH] D63387: clang: Promote -fdebug-compilation-dir from cc1 flag to driver-level flag

thakis created this revision. thakis added reviewers: hans, rnk. Herald added a subscriber: aprantl. The flag is useful when wanting to create .o files that are independent from the absolute path to the build directory. -fdebug-prefix-map= can be used to the same effect, but it requires putting th

[PATCH] D59744: Fix i386 ABI "__m64" type bug

thakis added a comment. In D59744#1547445 , @wxiao3 wrote: > @hans > > Please make sure all Chromium for 32-bit Linux libraries are following System > V ABI (i.e., m64 is passed on mmx register). I suspect that there are some > hand written assembly code

[PATCH] D64655: [Clang][Driver] don't error for unsupported as options for -no-integrated-as

thakis added a comment. Sorry, I missed this thread (and the other one) yesterday. Thanks for sorting out my mess; I'll think about what I want to do for the reland. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64655/new/ https://reviews.llvm.org

[PATCH] D58710: Added more detailed documentation for clangd

thakis added inline comments. Comment at: clang-tools-extra/trunk/docs/clangd/Installation.rst:325 + + $ ln -s ~/myproject/compile_commands.json ~/myproject-build/ + Isn't the order of the arguments the wrong way round here? Repository: rL LLVM CHANGES SINC

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

thakis added a comment. >> Those aren't diagnostics, so that's not surprising. > > What would suggest in that case? Add a new `-fpreprocessor-absolute-paths` > option? Or change the name of `-fdiagnostics-absolute-paths` for another name > that applies to both diagnostics and the preprocessor o

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

thakis added a comment. > So maybe `Driver::IsCLMode()` should take precedence over > `-fdiagnostics-absolute-paths` when using `/showIncludes`? Definitely not. We want builds to be deterministic; this includes them being independent of the build directory. (This matters for distributed compila

[PATCH] D64998: Improve clang-format-diff help output

thakis created this revision. thakis added a reviewer: krasimir. The description in clang-format-diff.py is more useful than the one in `clang-format-diff -h`, so use the same description in both places. https://reviews.llvm.org/D64998 Files: clang/tools/clang-format/clang-format-diff.py In

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

thakis added a comment. In D63648#1591469 , @aganea wrote: > It totally makes sense, thanks for the explanation Nico! Let's forget about > `cl` compatibility, that wasn't my initial intent. > > We always pass //relative// paths on the cmd-line, for all th

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

thakis created this revision. thakis added a reviewer: rnk. This relands r365703 (and r365714), originally reviewed at https://reviews.llvm.org/D64527. The problem with the old approach was that clang would now warn about -Wa flags that the integrated assembler didn't understand even when -fno-in

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

thakis added a comment. Note to self: Check https://github.com/ClangBuiltLinux/linux/issues/598 before landing this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65108/new/ https://reviews.llvm.org/D65108 ___ cfe-commits mailing list cfe-

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2051 + // Claim flags for the integrated assembler only if it's being used. + if (IsIntegratedAs) { +if (UseRelaxAll(C, Args)) (This jus

[PATCH] D64655: [Clang][Driver] don't error for unsupported as options for -no-integrated-as

thakis added a comment. I think https://reviews.llvm.org/D65108 is a better fix, for the reason rnk gave above. I think this one can be abandoned. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64655/new/ https://reviews.llvm.org/D64655

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2051 + // Claim flags for the integrated assembler only if it's being used. + if (IsIntegratedAs) { +if (UseRelaxAll(C, Args)) nickdesau

[PATCH] D65125: clang-format: Fix namespace end comments for namespaces with attributes and macros

thakis created this revision. thakis added reviewers: klimek, sammccall. Fixes PR39247. While here, also make C++20 `namespace A::inline B::inline C` nested inline namespaced definitions work. Before: #define DEPRECATE_WOOF [[deprecated("meow")]] namespace DEPRECATE_WOOF woof { void f(

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3564 +ArgStringList DummyArgs; +CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D, + TC.useIntegratedAs());

[PATCH] D65227: clang-format: Support `if CONSTEXPR` if CONSTEXPR is a macro.

thakis created this revision. thakis added a reviewer: klimek. This is like r305666 (which added support for `if constexpr`) except that it allows a macro name after the if. This is slightly tricky for two reasons: 1. r305666 didn't add test coverage for all cases where it added a kw_constexpr,

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3564 +ArgStringList DummyArgs; +CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D, + TC.useIntegratedAs());

[PATCH] D65000: [ARM] Set default alignment to 64bits

thakis added inline comments. Comment at: cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp:9 +// A16-NEXT: store <2 x i64> , <2 x i64>* [[BC]], align 16 +#include + This fails on some bots: http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/2

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

thakis abandoned this revision. thakis added a comment. We're going with D65233 instead. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65108/new/ https://reviews.llvm.org/D65108 ___ cfe-commits mailing list

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

thakis added a comment. In D64301#1602660 , @hubert.reinterpretcast wrote: > `ln -n` is not a standard option that is portably supported. I'll be > restoring the use of `rm` (with `-f` instead of `-rf`) and the associated > comment for removing `%t.fake

[PATCH] D65227: clang-format: Support `if CONSTEXPR` if CONSTEXPR is a macro.

thakis marked an inline comment as done. thakis added a comment. Thanks, landing with your suggestion applied. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65227/new/ https://reviews.llvm.org/D65227 ___ cfe-commits mailing list cfe-commits

[PATCH] D65233: driver: Don't warn about assembler flags being unused when not assembling; different approach

thakis added a comment. In D65233#1603741 , @bjope wrote: > I made another fixup (similar to https://reviews.llvm.org/rL367176) here > https://reviews.llvm.org/rL367182. > > Can someone please take a look (a post-commit review) of > https://reviews.llvm.

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

thakis added a comment. Thanks for sharing! (And for the fix, and apologies for breaking you.) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64301/new/ https://reviews.llvm.org/D64301 ___ cfe-commits mailing list cfe-

[PATCH] D65462: gn build: Fix check-clang-tools after r362702.

thakis created this revision. thakis added reviewers: hans, pcc, mbonadei. Herald added subscribers: llvm-commits, jfb. Herald added a reviewer: jdoerfert. Herald added a project: LLVM. r362702 added a test that requires clang-tidy to be linked into libclang, so add that to the gn build. https:/

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

thakis added a comment. Nice! This makes https://bugs.llvm.org/show_bug.cgi?id=42524#c3 easier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/ https://reviews.llvm.org/D65534 ___ cfe-commits

[PATCH] D65543: Use library basenames when autolinking on Windows

thakis added a comment. I kind of like the current behavior since it's consistent with how resource dirs are passed to cc1, and how __FILE__ expands if you give a relative work to clang, and how debug info paths look if you pass relative paths to clang. Repository: rG LLVM Github Monorepo C

[PATCH] D57165: [FileManager] Revert r347205 to avoid PCH file-descriptor leak.

thakis added a comment. Herald added a project: clang. A few weeks later, I realize that since the test doesn't have a ".test" extension, it doesn't run on any bots. Could you rename it to leakfiles.test (or any other fitting extension listed in clang/test/lit.cfg.py's config.suffixes list)?

[PATCH] D65956: clang: Diag running out of file handles while looking for files

thakis created this revision. thakis added a reviewer: rnk. clang would only print "file not found" when it's unable to find a header file. If the reason for that is a file handle leak, that's not a very useful error message. For errors that aren't in a small whitelist ("file not found", "file i

[PATCH] D52524: Add -Wno-poison-system-directories flag

thakis added a comment. Couldn't cross build users just pass -nostdsysteminc to tell clang to not look in system header locations? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 ___

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

thakis added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3559 // CollectArgsForIntegratedAssembler(). -if (TC.useIntegratedAs()) { +if (TC.useIntegratedAs() || isa(JA)) { Args.ClaimAllArgs(options::OPT_mrelax_all); We sh

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

thakis added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3559 // CollectArgsForIntegratedAssembler(). -if (TC.useIntegratedAs()) { +if (TC.useIntegratedAs() || isa(JA)) { Args.ClaimAllArgs(options::OPT_mrelax_all); dyung

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

thakis added a comment. Thanks! Looks like the change to the cpp file got lost? I only see the test change now. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65974/new/ https://reviews.llvm.org/D65974 ___ cfe-commit

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

thakis added a comment. Can you update the patch description to say what the patch does? With that, lgtm :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65974/new/ https://reviews.llvm.org/D65974 ___ cfe-commits ma

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

thakis added a comment. Something like "fix test for out-of-tree targets that don't enable the internal assembler by default by explicitly passing -fintegrated-as everywhere" or something like that. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65974/new/ https

<    1   2   3   4   5   6   7   8   9   10   >