[PATCH] D151188: [LLD][COFF] Add LLVM toolchain library paths by default.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaf744f0b84e2: [LLD][COFF] Add LLVM toolchain library paths by default. (authored by thieta). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D151188: [LLD][COFF] Add LLVM toolchain library paths by default.

thieta updated this revision to Diff 540344. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151188/new/ https://reviews.llvm.org/D151188 Files: clang/lib/Driver/Driver.cpp lld/COFF/Driver.cpp lld/COFF/Driver.h lld/docs/ReleaseNotes.rst

[PATCH] D155273: [clang-format] Add TypeNames option to disambiguate types/objects

thieta added a comment. Thanks for doing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155273/new/ https://reviews.llvm.org/D155273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D151188: [LLD][COFF] Add LLVM toolchain library paths by default.

thieta marked 6 inline comments as done. thieta added a comment. - Split the relative patch to this diff https://reviews.llvm.org/D155268 - Added some comments in both Clang and LLD - Fixed some code style. - Removed stray changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D151188: [LLD][COFF] Add LLVM toolchain library paths by default.

thieta updated this revision to Diff 540304. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151188/new/ https://reviews.llvm.org/D151188 Files: clang/lib/Driver/Driver.cpp lld/COFF/Driver.cpp lld/COFF/Driver.h

[PATCH] D151188: [LLD][COFF] Add LLVM toolchain library paths by default.

thieta updated this revision to Diff 540302. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151188/new/ https://reviews.llvm.org/D151188 Files: clang/lib/Driver/Driver.cpp lld/COFF/Driver.cpp lld/COFF/Driver.h

[PATCH] D151188: [LLD][COFF] Add LLVM toolchain library paths by default.

thieta updated this revision to Diff 540301. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151188/new/ https://reviews.llvm.org/D151188 Files: clang/lib/Driver/Driver.cpp

[PATCH] D154983: [clang-extdef-mapping] register necessary targest for ms-style asm block

thieta added a comment. Yes, I can do that, but I rather wait for @balazske to also approve this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154983/new/ https://reviews.llvm.org/D154983 ___

[PATCH] D154983: [clang-extdef-mapping] register necessary targest for ms-style asm block

thieta accepted this revision. thieta added a comment. This revision is now accepted and ready to land. Looks fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154983/new/ https://reviews.llvm.org/D154983

[PATCH] D150761: [NFC][Py Reformat] Reformat python files in clang and clang-tools-extra

This revision was automatically updated to reflect the committed changes. Closed by commit rGdd3c26a045c0: [NFC][Py Reformat] Reformat python files in clang and clang-tools-extra (authored by thieta). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150761: [NFC][Py Reformat] Reformat python files in clang and clang-tools-extra

thieta updated this revision to Diff 523680. thieta added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150761/new/ https://reviews.llvm.org/D150761 Files:

[PATCH] D150761: [NFC][Py Reformat] Reformat python files in clang and clang-tools-extra

thieta added inline comments. Comment at: clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py:9 # -#======# +#

[PATCH] D150761: [NFC][Py Reformat] Reformat python files in clang and clang-tools-extra

thieta created this revision. thieta added reviewers: jhenderson, JDevlieghere, MatzeB. Herald added subscribers: PiotrZSL, kadircet, mattd, asavonic, carlosgalvezp, abrachet, phosek, arphaman, fedor.sergeev, whisperity. Herald added a reviewer: NoQ. Herald added a reviewer: njames93. Herald

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

thieta added a comment. In D137327#4267342 , @MyDeveloperDay wrote: > You have commit rights correct? you really need to own your change especially > if it causes a regression. Alright - I did that now. Sorry I am just used to be on the other side as

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

thieta added a comment. Herald added a reviewer: rymiel. In D137327#4235255 , @MyDeveloperDay wrote: > In D137327#4234463 , @thieta wrote: > >> This was released in LLVM 16.0.0. > > The prior behaviour was there

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

thieta added a comment. This was released in LLVM 16.0.0. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137327/new/ https://reviews.llvm.org/D137327 ___ cfe-commits mailing list

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

thieta added a comment. In D137327#4233290 , @MyDeveloperDay wrote: > because of https://github.com/llvm/llvm-project/issues/61785 should this > really be reverted? is basically saying `X * Y {` must be `X *Y{` but that > obviously not the case

[PATCH] D143632: [clang] Handle __declspec() attributes in using

thieta added a comment. Thanks for the review and the explanations! I fixed your nits and landed this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143632/new/ https://reviews.llvm.org/D143632 ___

[PATCH] D143632: [clang] Handle __declspec() attributes in using

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG877859a09bda: [clang] Handle __declspec() attributes in using (authored by thieta). Changed prior to commit:

[PATCH] D143632: [clang] Handle __declspec() attributes in using

thieta marked an inline comment as done. thieta added a comment. Thanks for your comments - feel free to comment on the release note, I was struggling with describing the fix well for a short release note paragraph. Comment at: clang/lib/Parse/ParseDecl.cpp:61-63 + for

[PATCH] D143632: [clang] Handle __declspec() attributes in using

thieta updated this revision to Diff 496906. thieta marked 2 inline comments as done. thieta added a comment. - Expand on tests - Fix crash when Attrs was null - Added release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143632/new/

[PATCH] D138254: [llvm] Fix the build on OpenBSD by removing LLVM_VERSION_SUFFIX from created shared library names

thieta added a comment. @brad is this something you still need for OpenBSD? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138254/new/ https://reviews.llvm.org/D138254 ___ cfe-commits mailing list

[PATCH] D143632: [clang] Handle __declspec() attributes in using

thieta created this revision. thieta added reviewers: aaron.ballman, rsmith, tbaeder, saudi. Herald added a project: All. thieta requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch fixes so that declspec attributes are forwarded to

[PATCH] D139632: [clang-cl] Ignore #pragma managed / #pragma unmanaged

thieta added a comment. It still looks odd to me, but since the rest of the file has comments like that I am fine with it. But I think we should figure out if they are like that because some tool is parsing them or if it's just legacy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D139632: [clang-cl] Ignore #pragma managed / #pragma unmanaged

thieta added inline comments. Comment at: clang/lib/Lex/Pragma.cpp:1960 +/// "\#pragma managed" +/// "\#pragma managed(...)" I don't see the other comments in this file having `"\#` I think you can drop that. Also two `/` for the comments, not three.

[PATCH] D138254: [llvm] Fix the build on OpenBSD by removing LLVM_VERSION_SUFFIX from created shared library names

thieta added a comment. If this fixes OpenBSD it looks fine I think. But I wonder if we shouldn't just do this if we are on OpenBSD, changing the SOVERSION has been fraught with problems before since people have scripts that expect certain layouts. Without having more tests and more

[PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

thieta added a comment. I think this is ready to land @Mordante or is there anything else missing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137724/new/ https://reviews.llvm.org/D137724 ___

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

thieta added a comment. Makes sense they are not documented and we probably can ignore them in that case! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139167/new/ https://reviews.llvm.org/D139167 ___

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

thieta added a comment. I tried to look up what these options do exactly - but they don't seem to be documented. `/d1nodatetime` seems like an option that should be implemented instead of just ignored since it can have real reproducible problems not being there. Do you know what

[PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

thieta accepted this revision as: thieta. thieta added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: sstefan1, JDevlieghere. I think this is fine as we have discussed before. But I really dislike the code duplication for the check. We could put it in a include() I

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaa99b607b5cf: [clang][pdb] Dont include -fmessage-length in PDB buildinfo (authored by thieta). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

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

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

thieta updated this revision to Diff 473912. thieta added a comment. Updated from feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137322/new/ https://reviews.llvm.org/D137322 Files: clang/test/CodeGen/debug-info-codeview-buildinfo.c

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

thieta added a reviewer: hans. thieta added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137322/new/ https://reviews.llvm.org/D137322 ___ cfe-commits mailing list

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG70de684d4413: [clang-format] Handle object instansiation in if-statements (authored by thieta). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

thieta marked 2 inline comments as done. thieta added a comment. I have expanded testing and moved it to the place you suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137327/new/ https://reviews.llvm.org/D137327

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

thieta updated this revision to Diff 473169. thieta added a comment. Expand and move testing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137327/new/ https://reviews.llvm.org/D137327 Files: clang/lib/Format/TokenAnnotator.cpp

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

thieta created this revision. thieta added reviewers: MyDeveloperDay, curdeius, owenpan. Herald added a project: All. thieta requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Before this patch code like this: if (Class* obj{getObject()})

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

thieta added a comment. I posted https://reviews.llvm.org/D137322 to remove `-fmessage-length` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136474/new/ https://reviews.llvm.org/D136474 ___ cfe-commits

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

thieta created this revision. thieta added reviewers: aganea, rnk, thakis, aeubanks. Herald added a subscriber: hiraditya. Herald added a project: All. thieta requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. As discussed in

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

thieta added a comment. I am fine with this plan as well. And I am willing to chip in and try to fix the issues with the current implementation since we actually use the feature. @thakis do you mind explaining the issues you see and how to replicate them and I can have a look. Repository:

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

thieta added a comment. We have been building a two stage clang and our internal projects with this version of rpmalloc and clang 15.x a while now and I haven't see the issue you see @aganea. I don't think this patch is the problem as @efriedma suggested, could be in rpmalloc but since I have

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

thieta added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1345 while (CurrentToken) { -if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator)) +if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator)) {

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

thieta added a comment. Also this should have a release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136474/new/ https://reviews.llvm.org/D136474 ___ cfe-commits mailing list

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

thieta added a comment. We have tooling that uses this command line from the PDB. I think the conservative approach is to emit the command line by default since this is how MSVC behaves. I don't mind this flag now that I know about it, but I think we should keep the default to what MSVC does

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfd1d93db7106: [clang-format] Mark pragma region lines as StringLiterals (authored by thieta). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D136337: [clang-format] Discard pre-processor statements in parseBracedList()

thieta abandoned this revision. thieta added a comment. In D136337#3881476 , @owenpan wrote: > Can you create an issue on GitHub and include the details on how to reproduce > the problem using the latest clang-format? Ah I re-tested this with main and

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

thieta updated this revision to Diff 470380. thieta added a comment. git clang-format removed accidental include Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136336/new/ https://reviews.llvm.org/D136336 Files:

[PATCH] D136337: [clang-format] Discard pre-processor statements in parseBracedList()

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

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

thieta added a comment. In D136336#3877238 , @owenpan wrote: > Can you add a test to `TokenAnnotatorTest.cpp`? Thanks for the review. Fixed it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136336/new/

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

thieta updated this revision to Diff 470092. thieta added a comment. Added TokenAnnotatorTest and removed reduntant FormatTests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136336/new/ https://reviews.llvm.org/D136336 Files:

[PATCH] D136337: [clang-format] Discard pre-processor statements in parseBracedList()

thieta created this revision. thieta added reviewers: MyDeveloperDay, curdeius, owenpan. Herald added a project: All. thieta requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We had some code that looked like this: int foo() {

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

thieta created this revision. thieta added reviewers: MyDeveloperDay, curdeius, owenpan. Herald added a project: All. thieta requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In our code-base we auto-generate pragma regions the regions look

[PATCH] D135128: [clang][cli] Simplify repetitive macro invocations

thieta added a comment. This looks fine to me in principle. But I wonder if we should land the flag change first separately and make sure that no buildbots break because of it. Then we can merge the simplification a few days later when we are sure it's stabilized, since something similar

[PATCH] D134468: [Driver] Ignore -fmsc-version= -fms-compatibility-version= values smaller than 19

thieta added a comment. I think we should define a policy for this. I doubt we have many users on these older versions but I think maybe we should give deprecation messages for at least one release before we drop it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

thieta added a comment. I think the easiest way to handle this is that when you have it approved here - push it to fork on GitHub based on the release branch and create a GitHub issue and write /branch aballman/llvm-project/my_branch in a comment and it will queue up the cherry pick.

[PATCH] D133044: [Frontend] Restore Preprocessor::getPredefines()

thieta accepted this revision. thieta added a comment. LGTM and seems pretty safe so I am not opposed to merge it before 15 final. I would maybe add a link to the GitHub issue in the comment, but that's a nit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

thieta added a comment. Hmm. 15.0.0 is just a week away - I am not planning any more RCs unless we hit something critical. What's the risk of taking this specific change at this point? Would it make more sense to wait for 15.0.1? (I am guessing it's better if it goes into 15.0.0 or not in 15.x

[PATCH] D132791: Fix formatting in release notes

thieta accepted this revision. thieta added a comment. This revision is now accepted and ready to land. Thanks for doing this pass! Much appreciated. As long as you tried to build the docs after you changes and it passes without any warnings or errors I am happy for you to commit this directly

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

thieta added a comment. In D132486#3748546 , @thakis wrote: > Was this reviewed by anyone on the original change? As far as I can tell, > there was no agreement on the original change or here that reverting is the > way to go. Was this discussed

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

thieta added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:9-12 +else() + # ... unless explicily overridden + set(LIBCLANG_SOVERSION ${CLANG_VERSION_MAJOR}) +endif() h-vetinari wrote: > Sorry I didn't get to comment in time, but now that the

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG0f28d4856630: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION (authored by h-vetinari, committed by

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

thieta added a comment. @jrtc27 What do you think about this patch with the default flipped? I think this is how it should land personally as discussed on discourse. I have been trying to listen in how people want to handle this and I hope this is a good middle ground that we can agree on for

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

thieta updated this revision to Diff 455088. thieta added a comment. Fixed variable name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132486/new/ https://reviews.llvm.org/D132486 Files: clang/CMakeLists.txt clang/docs/ReleaseNotes.rst

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

thieta updated this revision to Diff 455087. thieta added a comment. Updated the option to default to ON - meaning we keep the current behavior found in main. This rationel for this is explained in my post here:

[PATCH] D132486: Revert "libclang.so: Make SONAME the same as LLVM version"

thieta created this revision. Herald added subscribers: fedor.sergeev, mgorny. Herald added a project: All. thieta requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This reverts commit bc39d7bdd4977a953b2e102f8f7eb479ad78984e

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

thieta added a comment. Hi @Sockke - this seems to be ready to land. Is there something holding it back? We ran into the same issue recently. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127293/new/ https://reviews.llvm.org/D127293 ___

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:57 + +* GCC >= 7.1 +* Clang >= 5.0 xbolva00 wrote: > Do we have bots which uses last supported compiler versions? GCC 7.1, Clang > 5.0 and MSVC 16.7. > > It is bad to promise something and

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. Hi - I tried to incorporate all the feedback here and added a post to discourse suggesting tweaks to the developer policy - please have a look and review it: https://discourse.llvm.org/t/rfc-updates-to-developer-policy-around-c-standards-bump/64383 Repository: rG

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. I think a week or two of moratorium would be good for sure. I think we can table this discussion for now and I will write a RFC post in discourse when I have time and we can discuss the details there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3709742 , @aaron.ballman wrote: > One thing I think would be a definite improvement is to have done an RFC on > Discourse for these changes so that downstreams have a chance to weigh in on > the impact. The patch was

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added inline comments. Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7 -set(MSVC_MIN 19.20) +set(MSVC_MIN 19.27) set(MSVC_SOFT_ERROR 19.27) glandium wrote: > thieta wrote: > >

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added inline comments. Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7 -set(MSVC_MIN 19.20) +set(MSVC_MIN 19.27) set(MSVC_SOFT_ERROR 19.27) glandium wrote: > You didn't update

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3706424 , @aaron.ballman wrote: > +1, thank you for thinking about how we can improve this process in the > future! Given that C++17 adoption across compilers has been far better than > C++20, I suspect the next time

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3706336 , @aaron.ballman wrote: > That's the only reason this hasn't been reverted already. Landing sweeping > changes on a weekend is a good way to reduce the pain, but we really need to > be sure someone watches

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3706263 , @aaron.ballman wrote: > > Something odd is going on here and we might want to consider a revert of this > patch until we resolve it. When I do a git pull and cmake files change, > Visual Studio's

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3706199 , @cor3ntin wrote: > Trying to read the logs,, notably > `C:\PROGRA~2\MIB055~1\2019\PROFES~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe > `, it would seem that this particular bot is running a version

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3705579 , @Jake-Egan wrote: > There is a failure on the AIX bot also: > ... > https://lab.llvm.org/buildbot/#/builders/214/builds/2707/steps/5/logs/stdio Filed an issue here:

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3705474 , @dyung wrote: > We are seeing an additional failure on an internal linux bot due to the > change to using C++17 by default when using GNU ld: > ... > Switching between BFD ld and gold still fails (although

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3705236 , @royjacobson wrote: > This affects people on their work branches as well, and it's not obvious that > it's a configuration error and not a broken master. > > The CMake approach sounds cleaner to me, but I

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3705131 , @royjacobson wrote: > This seems to have been more disruptive than expected, since an existing > CMakeCache.txt can make LLVM compile in previous C++14 configuration. This > seems to make some of the bots

[PATCH] D130689: [LLVM] Update C++ standard to 17

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb1356504e63a: [LLVM] Update C++ standard to 17 (authored by thieta). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta updated this revision to Diff 450364. thieta added a comment. Fixed spelling error and added links to C++ standard libraries Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130689/new/ https://reviews.llvm.org/D130689 Files:

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. All right! Last call - I am going fix the spelling error and merge this tomorrow EU time unless someone objects. Thanks for all reviews so far! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130689/new/

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. @nikic @thakis I fixed this issue in https://reviews.llvm.org/D131063 and it can be built with CXX_STANDARD=17 and OSX_DEPLOYMENT_TARGET=10.11. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130689/new/

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3690274 , @nikic wrote: > Given > https://github.com/llvm/llvm-project/blob/2bb7c54621f31a957302a4deb3d25b752acb07bd/llvm/include/llvm/Support/RWMutex.h#L22-L27, > it seems like this is supposed to be supported. This

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3689157 , @thakis wrote: > Is it expected and intentional that this increases the mac deployment target > to 10.12? I wasn't aware of that - but I think it's expected since the check in RWMutex checks for the C++

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3686723 , @h-vetinari wrote: > I don't think the standard library can be called a vendor-specific extension, > and so I think this still could/should be made clearer (even though the > status links below would show

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3686718 , @thieta wrote: > In D130689#3684399 , @h-vetinari > wrote: > >> It may be worth calling out that this is about C++17 core language and not >> the standard library?

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3684399 , @h-vetinari wrote: > It may be worth calling out that this is about C++17 core language and not > the standard library? > > libstdcxx only finished C++17 support in GCC 12, and libcxx is still missing >

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3686397 , @thakis wrote: > It'd be nice if this was landed opt-in behind some cmake variable at first, > so that folks could try it out on their bots and see how well things work. You can already test this with

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta updated this revision to Diff 448262. thieta added a comment. Fixed unintended indentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130689/new/ https://reviews.llvm.org/D130689 Files: bolt/runtime/CMakeLists.txt

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3684334 , @ChuanqiXu wrote: > So it is free that developers want to use some C++17 features in a small > amount of code, right? As soon as this land C++ 17 should be free to use everywhere yeah! Repository: rG

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3684360 , @barannikov88 wrote: > There are a few places (primarily in ADT and Support) that check __cplusplus > > 201402. Do they need to be cleaned up? Sounds good - but maybe that can be addressed in a separate

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta updated this revision to Diff 448261. thieta added a comment. Address some old comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130689/new/ https://reviews.llvm.org/D130689 Files: bolt/runtime/CMakeLists.txt clang/CMakeLists.txt

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta added a comment. In D130689#3684330 , @mehdi_amini wrote: > What does it mean exactly? We can't use **anything** C++17 without writing it > in the coding standards? > I'm not sure it'll be manageable: how do you see this playing out? Probably

[PATCH] D129446: [clang][driver] Find Apple default SDK path

thieta added a comment. Ping - any thoughts on this one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129446/new/ https://reviews.llvm.org/D129446 ___ cfe-commits mailing list

[PATCH] D130689: [LLVM] Update C++ standard to 17

thieta created this revision. thieta added reviewers: mehdi_amini, jyknight, jhenderson, hans, tstellar, cor3ntin, MaskRay. Herald added subscribers: ayermolo, StephenFan, mgorny. Herald added a reviewer: rafauler. Herald added a reviewer: Amir. Herald added a reviewer: maksfb. Herald added a

[PATCH] D129446: [clang][driver] Find Apple default SDK path

thieta created this revision. thieta added reviewers: egorzhdan, t.p.northover, dexonsmith, ldionne. Herald added a project: All. thieta requested review of this revision. Herald added a subscriber: MaskRay. Herald added a project: clang. Currently if you download clang and install it on macOS it

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta added inline comments. Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:42 + : Ctx(Context), SM(Context.getSourceManager()) { +CurrentFileName = astFilePath.str(); + } steakhal wrote: > Why is this not initialized in the

  1   2   >