Page MenuHomePhabricator

thieta (Tobias Hieta)
LLVM developer at Ubisoft Montreal

Projects

User does not belong to any projects.

User Details

User Since
Oct 18 2019, 11:34 AM (141 w, 4 d)

Recent Activity

Today

thieta accepted D129152: [Clang][unittests] Silence trucation warning with MSVC.

Thanks for switching to pragma!

Tue, Jul 5, 1:54 PM · Restricted Project, Restricted Project
thieta accepted D125744: github: Add a helpful message for issues without milestones.

Lgtm

Tue, Jul 5, 11:20 AM · Restricted Project, Restricted Project
thieta added a comment to D129152: [Clang][unittests] Silence trucation warning with MSVC.

Isn't it better to silence this warning with a pragma instead of disabling it for the whole file?

Tue, Jul 5, 10:45 AM · Restricted Project, Restricted Project
thieta added a comment to D129142: [LLD][ELF] Drop duplicates from rpath.

(Please don't land this until @MaskRay have had his say).

Tue, Jul 5, 6:28 AM · Restricted Project, Restricted Project, lld
thieta accepted D129142: [LLD][ELF] Drop duplicates from rpath.

LGTM - there shouldn't be that much use for duplicates here.

Tue, Jul 5, 6:27 AM · Restricted Project, Restricted Project, lld
thieta committed rGe6ff553979e8: [clang-extdef-mapping] Directly process .ast files (authored by thieta).
[clang-extdef-mapping] Directly process .ast files
Tue, Jul 5, 4:46 AM · Restricted Project, Restricted Project
thieta closed D128704: [clang-extdef-mapping] Directly process .ast files.
Tue, Jul 5, 4:46 AM · Restricted Project, Restricted Project
thieta updated the diff for D128704: [clang-extdef-mapping] Directly process .ast files.

Uppercase all variables

Tue, Jul 5, 2:12 AM · Restricted Project, Restricted Project
thieta added a comment to D128704: [clang-extdef-mapping] Directly process .ast files.

Ran some internal benchmarks on 1570 files (C and C++ mixed but much more C++ than C):

Tue, Jul 5, 1:00 AM · Restricted Project, Restricted Project

Sun, Jul 3

thieta added a comment to D128704: [clang-extdef-mapping] Directly process .ast files.

Thanks for the review! I uploaded a new version addressing all (I think) of your feedback and added a release note.

Sun, Jul 3, 2:11 AM · Restricted Project, Restricted Project
thieta updated the diff for D128704: [clang-extdef-mapping] Directly process .ast files.

Addressed review feedback.

Sun, Jul 3, 2:08 AM · Restricted Project, Restricted Project

Thu, Jun 30

thieta updated the diff for D128704: [clang-extdef-mapping] Directly process .ast files.

Clean-ups and error handling.

Thu, Jun 30, 12:05 AM · Restricted Project, Restricted Project

Wed, Jun 29

thieta added inline comments to D128704: [clang-extdef-mapping] Directly process .ast files.
Wed, Jun 29, 7:57 AM · Restricted Project, Restricted Project

Tue, Jun 28

thieta added inline comments to D128704: [clang-extdef-mapping] Directly process .ast files.
Tue, Jun 28, 3:56 AM · Restricted Project, Restricted Project
thieta updated the diff for D128704: [clang-extdef-mapping] Directly process .ast files.

Add test. Just repurpose the same test we already have but add a step to
generate ast first and then pushing that through extdef-mapping. It should
always produce the same result.

Tue, Jun 28, 3:51 AM · Restricted Project, Restricted Project
thieta accepted D128458: [llvm-lib] Ignore /SUBSYSTEM flag.

LGTM.

Tue, Jun 28, 12:23 AM · Restricted Project, Restricted Project
thieta committed rG3f0578dd87ee: [clang-cl] Add -emit-ast to clang-cl driver (authored by thieta).
[clang-cl] Add -emit-ast to clang-cl driver
Tue, Jun 28, 12:12 AM · Restricted Project, Restricted Project
thieta closed D128409: [clang-cl] Add -emit-ast to clang-cl driver.
Tue, Jun 28, 12:11 AM · Restricted Project, Restricted Project
thieta added a comment to D128704: [clang-extdef-mapping] Directly process .ast files.

This still needs tests - but I wanted to get your early input on the approach here and what you all think.

Tue, Jun 28, 12:10 AM · Restricted Project, Restricted Project
thieta requested review of D128704: [clang-extdef-mapping] Directly process .ast files.
Tue, Jun 28, 12:08 AM · Restricted Project, Restricted Project

Mon, Jun 27

thieta updated the summary of D128409: [clang-cl] Add -emit-ast to clang-cl driver.
Mon, Jun 27, 8:37 AM · Restricted Project, Restricted Project
thieta updated the diff for D128409: [clang-cl] Add -emit-ast to clang-cl driver.

Updated commit message

Mon, Jun 27, 8:37 AM · Restricted Project, Restricted Project
thieta updated the diff for D128409: [clang-cl] Add -emit-ast to clang-cl driver.

Just handle /o for AST and plist files

Mon, Jun 27, 8:36 AM · Restricted Project, Restricted Project
thieta added a comment to D128409: [clang-cl] Add -emit-ast to clang-cl driver.

Fixed the test issue. Regarding /Fo bit - if you want I can change it to only handle /o in this if statement. I don't mind either way.

Mon, Jun 27, 7:53 AM · Restricted Project, Restricted Project
thieta updated the diff for D128409: [clang-cl] Add -emit-ast to clang-cl driver.

Fixed missing -- in test file

Mon, Jun 27, 7:51 AM · Restricted Project, Restricted Project
thieta updated the diff for D128409: [clang-cl] Add -emit-ast to clang-cl driver.

Added tests and moved the check for TY_Plist and TY_AST to it's own if statement block

Mon, Jun 27, 5:20 AM · Restricted Project, Restricted Project

Thu, Jun 23

thieta updated the diff for D128409: [clang-cl] Add -emit-ast to clang-cl driver.

Removed unintentional whitespace removal and landed that as a NFC

Thu, Jun 23, 5:07 AM · Restricted Project, Restricted Project
thieta committed rGb6a33cec3830: [NFC] remove trailing whitespace (authored by thieta).
[NFC] remove trailing whitespace
Thu, Jun 23, 5:05 AM · Restricted Project, Restricted Project
thieta added a comment to D128409: [clang-cl] Add -emit-ast to clang-cl driver.

I'm unfamiliar with -emit-ast. Can you add some background on what this is for? What's CTU?

Thu, Jun 23, 5:01 AM · Restricted Project, Restricted Project
thieta added inline comments to D128409: [clang-cl] Add -emit-ast to clang-cl driver.
Thu, Jun 23, 1:43 AM · Restricted Project, Restricted Project
thieta requested review of D128409: [clang-cl] Add -emit-ast to clang-cl driver.
Thu, Jun 23, 1:42 AM · Restricted Project, Restricted Project

Tue, Jun 21

thieta accepted D128238: [LLD][COFF] Ignore /kernel flag.

Lgtm. Wait with merge until Hans checks it as well.

Tue, Jun 21, 11:20 AM · Restricted Project, Restricted Project
thieta added a comment to D128238: [LLD][COFF] Ignore /kernel flag.

I was just also about to comment that it seems like it's just ignoring the incremental flag and that it has to be more to it?

Tue, Jun 21, 6:23 AM · Restricted Project, Restricted Project

Thu, Jun 16

thieta added inline comments to D127938: Update Windows packaging script..
Thu, Jun 16, 11:46 PM · Restricted Project, Restricted Project

Wed, Jun 15

thieta requested changes to D127938: Update Windows packaging script..

A bunch of questions and comments. I think there are some really nice cleanups here, but also some features / behaviors that change that we need to understand.

Wed, Jun 15, 11:17 PM · Restricted Project, Restricted Project

Tue, Jun 14

thieta accepted D127709: [LLD][COFF] Convert file name to lowercase when inserting it into visitedLibs.

Thanks for the patch! Wait for Hans to give his approval as well before committing.

Tue, Jun 14, 12:51 PM · Restricted Project, Restricted Project
thieta added a comment to D127709: [LLD][COFF] Convert file name to lowercase when inserting it into visitedLibs.

Thanks for the test - but I am not sure it should be a part of nodefaultlib? Is there a reason you didn't add it as it's own test?

Tue, Jun 14, 12:14 PM · Restricted Project, Restricted Project
thieta added a comment to D127709: [LLD][COFF] Convert file name to lowercase when inserting it into visitedLibs.

Yep seems correct to me as well - but I think we need a test.

Tue, Jun 14, 4:31 AM · Restricted Project, Restricted Project

Fri, Jun 10

thieta accepted D127496: [NFC] Suggest Release mode in clang GettingStarted.html.

Looks fine to me. Thanks for fixing!

Fri, Jun 10, 10:19 AM · Restricted Project, Restricted Project
thieta requested changes to D127496: [NFC] Suggest Release mode in clang GettingStarted.html.

Thanks for fixing - but it's not completely accurate right now.

Fri, Jun 10, 7:21 AM · Restricted Project, Restricted Project

Thu, Jun 9

thieta added a comment to D125325: Pass plugin_name in SBProcess::SaveCore.

FYI: I helped @PatriosTheGreat to commit this after he asked for help in discord.

Thu, Jun 9, 7:31 AM · Restricted Project, Restricted Project
thieta committed rGa33983729df6: Pass plugin_name in SBProcess::SaveCore (authored by PatriosTheGreat).
Pass plugin_name in SBProcess::SaveCore
Thu, Jun 9, 7:30 AM · Restricted Project
thieta closed D125325: Pass plugin_name in SBProcess::SaveCore.
Thu, Jun 9, 7:30 AM · Restricted Project, Restricted Project

Tue, Jun 7

thieta accepted D127235: [NFC][test] Improve ecsymbols.test.

Much cleaner! Thanks!

Tue, Jun 7, 10:41 AM · Restricted Project, Restricted Project

Mon, Jun 6

thieta accepted D127135: [Object][Archive] Support a new archive member /<ECSYMBOLS>/.

I think you can drop the content lines from the test cases. But other that that it looks fine.

Mon, Jun 6, 12:36 PM · Restricted Project, Restricted Project

Jun 2 2022

thieta added a comment to D126940: github: Fix release automation /branch command with new repo.

This looks good - only a comment about error handling, but I think we can also address that later if we see it's a large problem.

Jun 2 2022, 11:04 PM · Restricted Project, Restricted Project
thieta accepted D126940: github: Fix release automation /branch command with new repo.
Jun 2 2022, 11:03 PM · Restricted Project, Restricted Project

Jun 1 2022

thieta committed rGfde9ef5214dc: [NFC][workflow] Fix issue where the workflow would say all PR's already exists (authored by thieta).
[NFC][workflow] Fix issue where the workflow would say all PR's already exists
Jun 1 2022, 11:20 PM · Restricted Project, Restricted Project

May 31 2022

thieta added a comment to D126699: [CMake] Skip linker check if the LLVM_LINKER_SKIP_TEST is set.

I think I rather would name the variable LLVM_LINKER_SKIP_TEST or something like that. I don't like the cmake variable name and since it's not something upstream we are adding I think I rather use a more descriptive name.

May 31 2022, 6:55 AM · Restricted Project, Restricted Project

May 27 2022

thieta committed rG49ad577c0772: [workflow] Don't fail workflow if we already have a PR for an issue (authored by thieta).
[workflow] Don't fail workflow if we already have a PR for an issue
May 27 2022, 7:03 AM · Restricted Project, Restricted Project
thieta closed D123657: [workflow] Don't fail workflow if we already have a PR for an issue.
May 27 2022, 7:02 AM · Restricted Project, Restricted Project

May 20 2022

thieta committed rG749fb33e82ff: [clang-format] Don't break lines after pragma region (authored by thieta).
[clang-format] Don't break lines after pragma region
May 20 2022, 7:11 AM · Restricted Project, Restricted Project
thieta closed D125961: [clang-format] Don't break lines after pragma region.
May 20 2022, 7:11 AM · Restricted Project, Restricted Project, Restricted Project
thieta updated the diff for D125961: [clang-format] Don't break lines after pragma region.

Removed comment

May 20 2022, 6:12 AM · Restricted Project, Restricted Project, Restricted Project
thieta updated the diff for D125961: [clang-format] Don't break lines after pragma region.

Switched to unit test over lit test

May 20 2022, 6:10 AM · Restricted Project, Restricted Project, Restricted Project
thieta added a comment to D125961: [clang-format] Don't break lines after pragma region.

Thanks for the patch!
Could you please add a unit test in unittest/Format/FormatTest.cpp instead of in lit-based test/?
Is there a bug report that your patch fixes?

May 20 2022, 5:01 AM · Restricted Project, Restricted Project, Restricted Project
thieta accepted D125924: [libcxx] Omit dllimport in public headers in MinGW mode.

Nice!

May 20 2022, 1:31 AM · Restricted Project, Restricted Project

May 19 2022

thieta added a comment to D125961: [clang-format] Don't break lines after pragma region.

There are other pragmas which include colon. How do they fare?

Example https://docs.microsoft.com/en-us/cpp/preprocessor/warning?view=msvc-170

#pragma warning( disable : 4507 34; once : 4385; error : 164 )
May 19 2022, 10:57 PM · Restricted Project, Restricted Project, Restricted Project
thieta updated the diff for D125961: [clang-format] Don't break lines after pragma region.

Added additional tests to check other pragma statements

May 19 2022, 10:56 PM · Restricted Project, Restricted Project, Restricted Project
thieta added a project to D125961: [clang-format] Don't break lines after pragma region: Restricted Project.
May 19 2022, 3:26 AM · Restricted Project, Restricted Project, Restricted Project
thieta updated the summary of D125961: [clang-format] Don't break lines after pragma region.
May 19 2022, 3:26 AM · Restricted Project, Restricted Project, Restricted Project
thieta requested review of D125961: [clang-format] Don't break lines after pragma region.
May 19 2022, 3:25 AM · Restricted Project, Restricted Project, Restricted Project

May 4 2022

thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Thanks everyone for your input! Let's hope this makes it easier for new users to pick the right defaults!

May 4 2022, 5:02 AM · Restricted Project, Restricted Project
thieta committed rG350bdf9227ce: [CMake] Make omitting CMAKE_BUILD_TYPE an error (authored by thieta).
[CMake] Make omitting CMAKE_BUILD_TYPE an error
May 4 2022, 5:01 AM · Restricted Project, Restricted Project
thieta closed D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.
May 4 2022, 5:01 AM · Restricted Project, Restricted Project
thieta committed rG30e879649603: [docs] Improve documentation around CMAKE_BUILD_TYPE (authored by thieta).
[docs] Improve documentation around CMAKE_BUILD_TYPE
May 4 2022, 2:24 AM · Restricted Project, Restricted Project
thieta closed D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.
May 4 2022, 2:23 AM · Restricted Project, Restricted Project
thieta added a comment to D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.

I will land this documentation update now since it's pretty low risk, and I want to land before the other diff making CMAKE_BUILD_TYPE mandatory. I'll be happy to make more changes if people feel it's needed. Hit me up with other changes in that case.

May 4 2022, 2:23 AM · Restricted Project, Restricted Project
thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Thanks for seeing this through!

Could you also update the CMake invocation in https://github.com/llvm/llvm-project/blob/be656df18721dc55a1de2eea64a3f73b6afa29a2/llvm/docs/GettingStarted.rst#getting-the-source-code-and-building-llvm? Or https://clang.llvm.org/get_started.html? There's probably going to be many more places like this.

May 4 2022, 2:21 AM · Restricted Project, Restricted Project
thieta updated the diff for D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Added CMAKE_BUILD_TYPE in several places in the documentation.

May 4 2022, 2:18 AM · Restricted Project, Restricted Project

May 3 2022

thieta updated the diff for D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Various grammar nits

May 3 2022, 1:16 AM · Restricted Project, Restricted Project
thieta added a comment to D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.

Thanks everyone for the input! If you have any more input on this - please try to get to it today since I plan to merge this and the CMake change to show this documentation tomorrow CEST.

May 3 2022, 12:50 AM · Restricted Project, Restricted Project
thieta updated the diff for D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Update URL to documentation that will point directly to CMAKE_BUILD_TYPE when https://reviews.llvm.org/D124367 has landed, which I plan to land at the same time.

May 3 2022, 12:49 AM · Restricted Project, Restricted Project
thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Added more detail to the release notes. Let me know if anyone else have any concerns - otherwise I plan to land this tomorrow morning CEST.

May 3 2022, 12:48 AM · Restricted Project, Restricted Project
thieta updated the diff for D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Added more details to Release Notes

May 3 2022, 12:47 AM · Restricted Project, Restricted Project

May 2 2022

thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

LGTM!

I applied the patch locally and tried it out with the MSVC built-in CMake support and confirmed that everything still works as I'd expect.

May 2 2022, 5:22 AM · Restricted Project, Restricted Project
thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Friendly ping on this one - I would love to get feedback positive or negative on this latest revision. I think it's important that we don't have anyone strongly objecting to this approach.

May 2 2022, 5:17 AM · Restricted Project, Restricted Project

May 1 2022

thieta added a comment to D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.

Thanks for all your detailed feedback @jhenderson

May 1 2022, 11:47 PM · Restricted Project, Restricted Project
thieta updated the diff for D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.

Latest batch up of updates

May 1 2022, 11:46 PM · Restricted Project, Restricted Project

Apr 29 2022

thieta updated the diff for D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Rewording.

Apr 29 2022, 7:37 AM · Restricted Project, Restricted Project
thieta retitled D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error from [CMake] Change default CMAKE_BUILD_TYPE to Release to [CMake] Make omitting CMAKE_BUILD_TYPE an error.
Apr 29 2022, 2:45 AM · Restricted Project, Restricted Project
thieta updated the diff for D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Make omitting CMAKE_BUILD_TYPE an error.

Apr 29 2022, 2:44 AM · Restricted Project, Restricted Project

Apr 28 2022

thieta updated the diff for D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.

Finally fix this line

Apr 28 2022, 11:30 PM · Restricted Project, Restricted Project
thieta added inline comments to D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.
Apr 28 2022, 3:25 AM · Restricted Project, Restricted Project
thieta updated the diff for D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.

Smaller grammar fixes

Apr 28 2022, 3:21 AM · Restricted Project, Restricted Project
thieta added inline comments to D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.
Apr 28 2022, 3:10 AM · Restricted Project, Restricted Project
thieta added a comment to D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.

Here is my latest attempt:

Apr 28 2022, 2:50 AM · Restricted Project, Restricted Project
thieta updated the diff for D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.

Review updates, make it a more readable table and point more people to
this table

Apr 28 2022, 2:49 AM · Restricted Project, Restricted Project
thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

I rarely, if ever, build the whole of LLVM on my Linux VM, because I don't need to, as I tend to work on only a small subset of the LLVM tools. This means that I rarely run into linker issues due to exhausted memory. I don't believe I ever bothered changing my linker default. Whilst my usage may not be the same as others, I want to raise it to highlight that the configuration isn't always broken - it mostly works just fine if you don't try and build everything at once.

Apr 28 2022, 1:48 AM · Restricted Project, Restricted Project
thieta accepted D124539: workflows: Add a test to ensure that the LLVM version is correct.
Apr 28 2022, 1:37 AM · Restricted Project, Restricted Project
thieta added a comment to D124539: workflows: Add a test to ensure that the LLVM version is correct.

I think this is fine. But I wonder if we should split out the version into a separate file that can be easier to parse and so we don't have to edit the version in several different places. In my last job we usually had a single VERSION file in the root that we read from CMake and all other places that needed the current version.

Apr 28 2022, 1:37 AM · Restricted Project, Restricted Project

Apr 27 2022

thieta accepted D124521: Remove the Visual Studio/MSBuild integration.
Apr 27 2022, 7:22 AM · Restricted Project, Restricted Project

Apr 26 2022

thieta added a comment to D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.

Thanks for the suggestions all - I am working on this and came up with this table, WDYT. I will upload this as a diff when I have addressed all the rest of the comments, but wanted to get the input on the table first since it's a bit annoying to mark-up correctly.

Apr 26 2022, 9:57 AM · Restricted Project, Restricted Project
thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

I'm OK with #1 and #2, but note that the combo in #3 is not ALWAYS broken, just if you don't have enough memory or are running too many link jobs at the same time. I'm not sure we want it to be an 'error'.

Apr 26 2022, 9:40 AM · Restricted Project, Restricted Project

Apr 25 2022

thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

One thing we all seem to agree on is that documentation could be better - so I took a stab at that here - I think it's connected to this discussion, but I don't think it's the only fix, please see the doc suggestions here: https://reviews.llvm.org/D124367

Apr 25 2022, 1:35 AM · Restricted Project, Restricted Project
thieta requested review of D124367: [docs] Improve documentation around CMAKE_BUILD_TYPE.
Apr 25 2022, 1:31 AM · Restricted Project, Restricted Project

Apr 22 2022

thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Rather than changing the default and annoying or confusing people, improve the documentation! Have cmake print a link to the awesome documentation.

Apr 22 2022, 9:15 AM · Restricted Project, Restricted Project
thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

I think both of the situations are basically the same with basically the same outcomes -- if the user picks the "correct" build target (either through the default or explicitly), they're happy, if not, they're unhappy.

Apr 22 2022, 6:00 AM · Restricted Project, Restricted Project
thieta added a comment to D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Personally, I think the default depends entirely on who is doing the download and from where. If people are doing a git clone of the main branch, I think they're more often developers and would want a debug build and switching to release will be a burden/surprise/annoyance. If they're snagging source from the release downloads page, they're probably not doing active development with that and would want a release build of some kind and the current behavior is a burden/surprise/annoyance. Without some measurements of how users are getting to the source in the first place, I don't think we can reasonably pick a default for all situations and call it better than the current one. It'll just be different.

Apr 22 2022, 5:47 AM · Restricted Project, Restricted Project

Apr 21 2022

thieta updated the diff for D124153: [CMake] Make omitting CMAKE_BUILD_TYPE an error.

Added release note about assertions.

Apr 21 2022, 8:48 AM · Restricted Project, Restricted Project