thakis (Nico Weber)Email Not Verified
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 10 2013, 2:43 PM (236 w, 3 h)

Recent Activity

Mon, Jul 17

thakis closed D35236: Refactor gdb index creation.

(landed in r307867)

Mon, Jul 17, 5:26 AM

Fri, Jul 14

thakis added inline comments to D35379: Add documentation for @available.
Fri, Jul 14, 11:53 AM
thakis added a comment to D35379: Add documentation for @available.

I went ahead and landed this in r308044, given that I addressed the nits and removed the possibly contentious bits. Happy to address remaining nits in a follow-up.

Fri, Jul 14, 11:42 AM
thakis added a comment to D35379: Add documentation for @available.

Mostly done, thanks!

Fri, Jul 14, 9:26 AM
thakis updated the diff for D35379: Add documentation for @available.

sdy comments

Fri, Jul 14, 9:26 AM
thakis updated the diff for D35379: Add documentation for @available.

arphaman comments

Fri, Jul 14, 9:17 AM
thakis added a comment to D35379: Add documentation for @available.

Thanks, all done, much better!

Fri, Jul 14, 9:16 AM

Thu, Jul 13

thakis updated the diff for D35379: Add documentation for @available.

Document *, add example with multiple platforms

Thu, Jul 13, 1:58 PM
thakis added inline comments to D35379: Add documentation for @available.
Thu, Jul 13, 1:35 PM
thakis updated the diff for D35379: Add documentation for @available.

comments

Thu, Jul 13, 1:35 PM
thakis created D35379: Add documentation for @available.
Thu, Jul 13, 1:06 PM

Tue, Jun 27

thakis closed D34637: [libunwind] Add _LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS.

r306442, thanks!

Tue, Jun 27, 11:37 AM

May 25 2017

thakis added a comment to D32862: [AMDGPU] add intrinsic for s_getpc.

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/2604
http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/3280

May 25 2017, 12:20 PM

May 24 2017

thakis added a comment to D33471: [asan] Remove allow_user_segv_handler on Windows..

Either this or the reland of making allow_user_segv_handler to true broke this test on Windows:

May 24 2017, 1:06 PM
thakis added a comment to D33408: MachineCSE: Respect interblock physreg liveness.

The new test fails for me like so, at r303736:

May 24 2017, 11:59 AM

May 10 2017

thakis accepted D26953: clang-format: handle formatting on constexpr if.

This looks good to me, thanks. Sorry about the slow turnaround. Do you have commit access? If not, I can land it for you – but it also looks like you've contributed several patches by now, so you could also ask for commit access if you don't have it yet.

May 10 2017, 8:27 AM

May 5 2017

thakis closed D32879: Warn that the [] spelling of uuid(...) is deprecated..

r302255, thanks!

May 5 2017, 10:19 AM
thakis added a comment to D32914: Introduce Wzero-as-null-pointer-constant..

s/Add one/All done/

May 5 2017, 9:24 AM
thakis closed D32914: Introduce Wzero-as-null-pointer-constant..

Thanks! Add one and landed in r302247.

May 5 2017, 9:24 AM
thakis created D32914: Introduce Wzero-as-null-pointer-constant..
May 5 2017, 8:58 AM

May 4 2017

thakis created D32879: Warn that the [] spelling of uuid(...) is deprecated..
May 4 2017, 1:55 PM

May 1 2017

thakis added inline comments to D32646: Fix a bug that -Wmissing-braces fires on system headers.
May 1 2017, 2:41 PM

Apr 24 2017

thakis created D32435: clang-cl: Add support for /permissive-.
Apr 24 2017, 7:58 AM

Apr 21 2017

thakis closed D32371: Add comments to the diagnostic kinds in Diagnostic.td..

r301039, thanks.

Apr 21 2017, 2:08 PM
thakis added a comment to D32371: Add comments to the diagnostic kinds in Diagnostic.td..

Not sure if this is useful, but this is where I always check first.

Apr 21 2017, 2:00 PM
thakis created D32371: Add comments to the diagnostic kinds in Diagnostic.td..
Apr 21 2017, 2:00 PM
thakis closed D32369: [ms] Give -Wmicrosoft-enum-forward-reference a chance to fire in clang-cl, PR32736.

r301032, thanks.

Apr 21 2017, 1:25 PM
thakis created D32369: [ms] Give -Wmicrosoft-enum-forward-reference a chance to fire in clang-cl, PR32736.
Apr 21 2017, 1:17 PM

Apr 19 2017

thakis added inline comments to D32095: Make empty shell of new cvtres tool..
Apr 19 2017, 1:17 PM
thakis accepted D32095: Make empty shell of new cvtres tool..

Looks good to me.

Apr 19 2017, 1:08 PM
thakis closed D27375: Let ubsan search UBSAN_SYMBOLIZER_PATH for llvm-symbolizer.

I was just confused again by this not working, and didn't remember this review for a while. From a user's point of view it feels like this should work, so I've gone ahead and landed this in r300692.

Apr 19 2017, 7:17 AM

Apr 18 2017

thakis added a comment to D32095: Make empty shell of new cvtres tool..

I'm out today. Since lld will be a client of the library code of this tool, Rui should probably take a look if he's around this week. I'd also already add a test with just a RUN line for running the tool without parameters, just to have test coverage from day one (makes it easier to add tests in the future once the tool actually does something).

Apr 18 2017, 6:32 AM
thakis added a reviewer for D32095: Make empty shell of new cvtres tool.: ruiu.
Apr 18 2017, 6:31 AM

Apr 11 2017

thakis closed D31652: [clang-format] Recognize Java logical shift assignment operator.

Landed in r299952: http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20170410/189882.html

Apr 11 2017, 9:03 AM
thakis accepted D31652: [clang-format] Recognize Java logical shift assignment operator.

Looks good. Do you have commit access?

Apr 11 2017, 8:19 AM
thakis added a comment to D29586: [lsan] Enable LSan for arm Linux.

This breaks bootstrap builds. I put the error message in the revert commit description: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170410/444101.html

Apr 11 2017, 7:42 AM · Restricted Project

Apr 6 2017

thakis added inline comments to D31652: [clang-format] Recognize Java logical shift assignment operator.
Apr 6 2017, 6:41 AM

Mar 27 2017

thakis added a comment to D30504: [sanitizer] Bail out with warning if user dlopens shared library with RTLD_DEEPBIND flag.

We have a test in chromium that loads a small so file and calls some of the functions therein. There are no allocations in the so file. The test used to run fine under tsan, now it fails with the error message you added. I'll disable the test under tsan, but maybe others out there where also calling non-allocating so's without problems before this change.

Mar 27 2017, 7:44 AM · Restricted Project

Mar 22 2017

thakis accepted D31248: [X86] Implement __readgsqword (and the rest) as builtins (PR32373).
Mar 22 2017, 11:48 AM

Mar 18 2017

thakis closed D30959: [pdb] Add support for writing Module Info and module symbols.

I think this landed in r297900.

Mar 18 2017, 9:06 AM

Mar 16 2017

thakis added a comment to D30991: [Driver] Fix cross compiling with Visual Studio 2017.

Ah ok, thanks for explaining. In that case, this sounds fine and I'll leave the review to zturner.

Mar 16 2017, 11:27 AM
thakis added a comment to D30991: [Driver] Fix cross compiling with Visual Studio 2017.

When you say "cross-compiling", you mean targeting Windows while running on non-Windows, right? How do dlls get loaded there at all?

Mar 16 2017, 11:06 AM
thakis added a comment to D30991: [Driver] Fix cross compiling with Visual Studio 2017.

What env vars are needed here? Reading an env file seems a bit inelegant, could we pass the values of these env vars as flags instead?

Mar 16 2017, 10:55 AM

Mar 14 2017

thakis added a comment to D30923: Warn on enum assignment to bitfields that can't fit all values.

Maybe it should have some "to suppress the warning, do X" fixit?

Mar 14 2017, 8:45 AM
thakis accepted D30923: Warn on enum assignment to bitfields that can't fit all values.

Looks like a cool warning. Two suggestions for more exhaustive testing, but I think this looks good.

Mar 14 2017, 8:44 AM

Mar 13 2017

thakis added inline comments to D30923: Warn on enum assignment to bitfields that can't fit all values.
Mar 13 2017, 6:09 PM

Mar 3 2017

thakis added a comment to D30593: Add correct "-isystem"/"-isysroot" warning handling to static analysis' BugReporter..

http://llvm-cs.pcc.me.uk/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h/risInSystemHeader suggests that the analyzer has some plumbing for this, so I added dcoughlin as reviewer, who has touched some of those lines before. dcoughlin, as background: We're playing with running the analyzer on chromium, and we were pretty surprised that it defaults to printing diagnostics for system headers. That's different from what regular clang does, and there isn't much applications can do about diagnostics in system headers. kmarshall wrote a script to manually filter out diagnostics from system headers, but we figured it'd make more sense if the analyzer didn't emit those diagnostics in the first place -- probably by default, but maybe behind some flag. Are you familiar with the design behind the current behavior? Does it make sense to change this? Are we missing some existing flag?

Mar 3 2017, 3:26 PM
thakis added a reviewer for D30593: Add correct "-isystem"/"-isysroot" warning handling to static analysis' BugReporter.: dcoughlin.
Mar 3 2017, 3:23 PM

Mar 1 2017

thakis added a comment to D30326: [MS-ABI] Allow #pragma section to choose for ZI data.

Adding incompatible extensions to MS extensions sounds like bad precedent to me. I think it'd be good if a code owner could opine on this change before it goes in.

Mar 1 2017, 8:40 AM

Feb 28 2017

thakis added a comment to D30326: [MS-ABI] Allow #pragma section to choose for ZI data.

I'm surprised this is behind a flag. This pragma attempts to be compatible with cl.exe. Does cl.exe do this by default? If so, we should do it by default. If not, why add this as an incompatible thing to an MS extension instead of keying it off some attribute that works in non-MS mode too?

Feb 28 2017, 8:20 AM

Feb 27 2017

thakis closed D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded.

r296408, thanks!

Feb 27 2017, 3:13 PM
thakis closed D27455: UBSan docs: Explicitly mention that `-fsanitize=unsigned-integer-overflow` does not catch UB..

296387.

Feb 27 2017, 1:39 PM
thakis added inline comments to D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded.
Feb 27 2017, 1:22 PM
thakis updated the diff for D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded.

Here's a simpler fix, suggested by rnk.

Feb 27 2017, 1:20 PM
thakis added inline comments to D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded.
Feb 27 2017, 6:47 AM

Feb 26 2017

thakis added a comment to D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded.

(related, and why i picked rnk as reviewer: https://bugs.llvm.org/show_bug.cgi?id=17216 https://bugs.llvm.org/show_bug.cgi?id=17960)

Feb 26 2017, 5:06 PM
thakis created D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded.
Feb 26 2017, 4:56 PM
thakis added a comment to D27455: UBSan docs: Explicitly mention that `-fsanitize=unsigned-integer-overflow` does not catch UB..

samsonov: ping

Feb 26 2017, 4:49 PM

Feb 10 2017

thakis added a comment to D28543: Eliminates uninitialized warning for volatile variables..

What's the motivation for this? The placement of a local volatile variable is still under the compiler's direction, and unless the address escapes, we still assume we can reason about its aliasing (and, thus, whether or not it is initialized).

Feb 10 2017, 1:04 PM

Jan 31 2017

thakis closed D29337: Keep Chromium ObjC column limit at 80 for consistency with C++.

r293675

Jan 31 2017, 10:53 AM
thakis accepted D29337: Keep Chromium ObjC column limit at 80 for consistency with C++.
Jan 31 2017, 10:38 AM

Jan 13 2017

thakis accepted D28543: Eliminates uninitialized warning for volatile variables..

Looks great to me, thanks! Unless someone else shouts, I'll land this for you soon.

Jan 13 2017, 12:41 PM

Jan 11 2017

thakis added a comment to D28543: Eliminates uninitialized warning for volatile variables..

Thanks, this is looking pretty good! From clicking around a bit on cs, do you think it's better to put the check where you have it, or is maybe http://llvm-cs.pcc.me.uk/tools/clang/lib/Analysis/UninitializedValues.cpp#36 more appropriate? I think having it where you have it means saying "we can reason about volatile vars, and we want to treat them as initialized" while the other spot means "we can't reason about volatile variables at all". I don't know which place is better (of someone else reading this knows, please speak up). If the other place makes your test fail, having the check where you have it is probably fine. Else I'd say that moving the check up is probably better.

Jan 11 2017, 3:00 PM
thakis added a comment to D28543: Eliminates uninitialized warning for volatile variables..

Thanks for the test!

Jan 11 2017, 1:42 PM

Jan 10 2017

thakis added a comment to D28543: Eliminates uninitialized warning for volatile variables..

Thanks! Can you add a test somewhere? grep -R W.*uninitialized test/Sema* will probably show you the existing test for this code. (Another technique: Comment some of the existing tests, run ninja check-clang, and see which tests start failing.)

Jan 10 2017, 6:18 PM
thakis added a comment to D20136: Get default -fms-compatibility-version from cl.exe's version.

Aha, looks like this did have a #pragma comment(lib when it went in (for version.lib which is actually the correct one), but takumi removed it in http://llvm.org/viewvc/llvm-project?rev=269557&view=rev

Jan 10 2017, 7:34 AM

Jan 9 2017

thakis added a comment to D20136: Get default -fms-compatibility-version from cl.exe's version.

One consequence from this that I just realized is that linking a binary depending on clang stuff with (morally):

Jan 9 2017, 3:29 PM

Jan 3 2017

thakis closed D28165: Change clang-format's Chromium JavaScript defaults.

http://llvm.org/viewvc/llvm-project?view=revision&revision=290930

Jan 3 2017, 6:52 PM
thakis accepted D28165: Change clang-format's Chromium JavaScript defaults.

I'd rather we don't deviate from google style at all, but this brings us closer from where we are to that, so lgtm :-)

Jan 3 2017, 2:26 PM
thakis added inline comments to D28165: Change clang-format's Chromium JavaScript defaults.
Jan 3 2017, 9:38 AM

Dec 19 2016

thakis closed D27769: Update MSVC compat docs about debug info..

r289712

Dec 19 2016, 2:52 PM

Dec 18 2016

thakis accepted D27909: Add a lit test for PR31374.

Thanks!

Dec 18 2016, 6:46 PM

Dec 16 2016

thakis added a comment to D27858: Default to standalone debug info on Windows.

Did you measure what this does to link times?

Dec 16 2016, 11:59 AM

Dec 15 2016

thakis added inline comments to D25538: Make lsan complain loudly when running under ptrace.
Dec 15 2016, 11:29 PM
thakis added a comment to D27827: [ObjC] CodeGen support for @available on macOS.

Huh, I'm surprised this has a runtime component, I missed that. I thought this would be a statically-checked thing. But having this does seem useful :-)

Dec 15 2016, 3:13 PM
thakis added a comment to D27659: [sanitizer] intercept bstring functions.
In D27659#623405, @kubabrecka wrote:

There's nothing wrong with this patch, but it's a binary-compatibility issue

What is the issue with ABI?

A system library requires that bzero doesn't touch some specific register. This is true for the current system implementation of bzero, but not when using the interceptor.

Dec 15 2016, 11:46 AM

Dec 14 2016

thakis updated the diff for D27769: Update MSVC compat docs about debug info..

reword slightly

Dec 14 2016, 1:08 PM
thakis added inline comments to D27769: Update MSVC compat docs about debug info..
Dec 14 2016, 1:07 PM
thakis updated the diff for D27769: Update MSVC compat docs about debug info..

no dwarf or gcc mode

Dec 14 2016, 12:54 PM
thakis added inline comments to D27769: Update MSVC compat docs about debug info..
Dec 14 2016, 12:52 PM
thakis added inline comments to D27769: Update MSVC compat docs about debug info..
Dec 14 2016, 12:32 PM
thakis retitled D27769: Update MSVC compat docs about debug info. from to Update MSVC compat docs about debug info..
Dec 14 2016, 12:32 PM

Dec 9 2016

thakis closed D27545: Don't assert when redefining a built-in macro in a PCH, PR29119.

r289228, thanks!

Dec 9 2016, 9:43 AM
thakis added a comment to D27545: Don't assert when redefining a built-in macro in a PCH, PR29119.

(The reason that the predefines buffer doesn't override the value of __STDC_HOSTED__ from the pch is that CompilerInstance::createPCHExternalASTSource overrides the predefines buffer with a different predefines buffer suggested by the ASTReader, usually an empty one.)

Dec 9 2016, 3:59 AM
thakis updated the diff for D27545: Don't assert when redefining a built-in macro in a PCH, PR29119.

update a comment

Dec 9 2016, 3:59 AM
thakis added inline comments to D27545: Don't assert when redefining a built-in macro in a PCH, PR29119.
Dec 9 2016, 3:59 AM

Dec 7 2016

thakis added a comment to D27545: Don't assert when redefining a built-in macro in a PCH, PR29119.

Thanks!

Dec 7 2016, 4:25 PM
thakis updated the diff for D27545: Don't assert when redefining a built-in macro in a PCH, PR29119.

comments

Dec 7 2016, 4:25 PM
thakis retitled D27545: Don't assert when redefining a built-in macro in a PCH, PR29119 from to Don't assert when redefining a built-in macro in a PCH, PR29119.
Dec 7 2016, 2:07 PM

Dec 6 2016

thakis retitled D27455: UBSan docs: Explicitly mention that `-fsanitize=unsigned-integer-overflow` does not catch UB. from to UBSan docs: Explicitly mention that `-fsanitize=unsigned-integer-overflow` does not catch UB..
Dec 6 2016, 6:13 AM

Dec 2 2016

thakis added a comment to D27375: Let ubsan search UBSAN_SYMBOLIZER_PATH for llvm-symbolizer.

Works for me too. After reading the code, I think setting (ASAN|UBSAN|MSAN)_OPTIONS=external_symbolizer_path=foo should already work too. But all the docs mention ASAN_SYMBOLIZER_PATH instead. Should we have LLVM_SYMBOLIZER_PATH in addition to the FOO_OPTIONS=external_symbolizer_path=foo flag that already exists, or is that enough? Should the docs mention that over ASAN_SYMBOLIZER_PATH and MSAN_SYMBOLIZER_PATH? Should the runtime print "use FOO_OPTIONS=external_symbolizer_path=foo instead of ASAN_SYMBOLIZER_PATH" if it's set so that it can eventually be removed?

Dec 2 2016, 4:17 PM
thakis retitled D27375: Let ubsan search UBSAN_SYMBOLIZER_PATH for llvm-symbolizer from to Let ubsan search UBSAN_SYMBOLIZER_PATH for llvm-symbolizer.
Dec 2 2016, 4:05 PM
thakis added a comment to D27279: Store decls in prototypes on the declarator instead of in the AST.

(Unrelated, but if you're looking at memory: When I was looking at it a while ago, IIRC a surprising amount of memory was taken up by CXXBasePaths objects, and just reordering fields to pack it better made that object several bytes smaller. IIRC I accidentally reverted my local patch for this, but if you're in the mood it's probably not too hard to recreate)

Dec 2 2016, 12:48 PM

Nov 28 2016

thakis accepted D27176: cmake: Set rpath for loadable modules as well as shared libraries..

Might want to mention that this affects LLVMgold.so in the patch description.

Nov 28 2016, 2:07 PM

Nov 25 2016

thakis added a comment to D27098: [ELF] Refactor EhOutputSection.

This breaks building on macOS:

Nov 25 2016, 8:40 AM · lld

Nov 22 2016

thakis closed D26984: Unconditionally pass -lto_library, remove -Wliblto warning..

287685, thanks!

Nov 22 2016, 11:48 AM
thakis retitled D26984: Unconditionally pass -lto_library, remove -Wliblto warning. from to Unconditionally pass -lto_library, remove -Wliblto warning..
Nov 22 2016, 11:32 AM

Nov 11 2016

thakis added a comment to D26540: [Sema] Accept and ignore the leaf attribute.

(Maybe implementing real support isn't so difficult? From a quick glance, it sounds like this is a sema-only attribute?)

Nov 11 2016, 7:45 AM

Nov 10 2016

thakis closed D26163: [clang-format] Fix PR30527: Regression when clang-format insert spaces in [] when in template.

Landed in r286507. Thanks!

Nov 10 2016, 1:59 PM

Nov 6 2016

thakis retitled D26335: [ms] Reinstate https://reviews.llvm.org/D14748 after https://reviews.llvm.org/D20291 from to [ms] Reinstate https://reviews.llvm.org/D14748 after https://reviews.llvm.org/D20291.
Nov 6 2016, 1:06 PM