smeenai (Shoaib Meenai)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 19 2016, 10:21 AM (92 w, 1 d)

Recent Activity

Fri, May 25

smeenai added inline comments to D47391: [COFF] Don't crash when emitting symbol table for SafeSEH input files.
Fri, May 25, 3:43 PM
smeenai added inline comments to D47391: [COFF] Don't crash when emitting symbol table for SafeSEH input files.
Fri, May 25, 1:47 PM
smeenai created D47391: [COFF] Don't crash when emitting symbol table for SafeSEH input files.
Fri, May 25, 1:47 PM

Wed, May 23

smeenai added a comment to D47249: [ELF] Implement --icf=safe.

See also @pcc's proposal: http://lists.llvm.org/pipermail/llvm-dev/2018-May/123514.html

Wed, May 23, 11:19 AM

Tue, May 22

smeenai updated the diff for D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.

Colocate CHECK lines with declarations

Tue, May 22, 4:59 PM
smeenai created D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.
Tue, May 22, 3:44 PM

Fri, May 18

smeenai committed rC332777: [test] Fix run line to use correct triple.
[test] Fix run line to use correct triple
Fri, May 18, 3:03 PM
smeenai committed rL332777: [test] Fix run line to use correct triple.
[test] Fix run line to use correct triple
Fri, May 18, 3:03 PM
smeenai added a comment to D47083: Enable colored diagnostics when building with gcc 4.9+..

Why do we hardcode compiler IDs/versions here instead of just checking whether the current compiler supports the flag?

Fri, May 18, 12:33 PM

Tue, May 15

smeenai committed rL332430: [ObjCARC] Prevent code motion into a catchswitch.
[ObjCARC] Prevent code motion into a catchswitch
Tue, May 15, 9:59 PM
smeenai closed D46482: [ObjCARC] Prevent code motion into a catchswitch.
Tue, May 15, 9:59 PM
smeenai updated the diff for D46482: [ObjCARC] Prevent code motion into a catchswitch.

@ahatanak's suggestion

Tue, May 15, 2:38 PM
smeenai added inline comments to D46482: [ObjCARC] Prevent code motion into a catchswitch.
Tue, May 15, 2:10 PM

Sun, May 13

smeenai added a comment to D46482: [ObjCARC] Prevent code motion into a catchswitch.

Ping.

Sun, May 13, 6:23 PM

Fri, May 11

smeenai added a comment to D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers.

Emailed cfe-dev: http://lists.llvm.org/pipermail/cfe-dev/2018-May/057934.html

Fri, May 11, 4:29 PM
smeenai added inline comments to D46672: COFF: ICF a section and its associated pdata section as a unit..
Fri, May 11, 11:22 AM

Wed, May 9

smeenai added a comment to D46648: [SimplifyLibcalls] Optimize string concats using s(n)printf.

If you're not planning to move forward with the diff, it's better to abandon it rather than close it. Close usually implies it was committed.

Wed, May 9, 2:57 PM
smeenai added a comment to D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers.

Yeah, I think we all agree now that a portability warning isn't really tractable. Note that even for the warnings that motivated this diff, they should have only fired if size_t and NSInteger had separate types, so it wasn't a portability warning in that sense to begin with (as in, it would only warn if there was a mismatch for your current target, not if there was a potential mismatch for any target).

Wed, May 9, 1:25 PM

Tue, May 8

smeenai added a comment to D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers.

I agree that the format-specifier checker is not intended to be a portability checker.

Any attempt to check portability problems has to account for two things:

  • Not all code is intended to be portable. If you're going to warn about portability problems, you need some way to not annoy programmers who might have good reason to say that they only care about their code running on Linux / macOS / 64-bit / 32-bit / whatever. Generally this means splitting the portability warning out as a separate warning group.
  • There are always established idioms for solving portability problems. In this case, a certain set of important typedefs from the C standard have been blessed with dedicated format modifiers like "z", but every other typedef in the world has to find some other solution, either by asserting that some existing format is good enough (as NSInteger does) or by introducing a macro that expands to an appropriate format (as some things in POSIX do). A proper format-portability warning would have to know about all of these conventions (in order to check that e.g. part of the format string came from the right macro), which just seems wildly out-of-scope for a compiler warning.
Tue, May 8, 3:27 PM
smeenai updated subscribers of D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers.
In D42933#1091817, @jfb wrote:

I also think that special casing these two specifiers doesn't make sense. The problem is a general issue -- and one I've often found irritating. This exact same situation comes up all the time in non-Darwin contexts too.

I don't think that's true. In this very specific case the platform guarantees that (sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == sizeof(NSUInteger)) for all architectures this platform supports. This exact same situation does not come up all the time in other contexts because the int / long / long long distinction isn't backed by a portability guarantee. The warning is there to say "this code isn't portable!", but in the very specific case of NSInteger and NSUInteger it is and users rely on it so it cannot be broken. The warning is therefore spurious, users therefore rightly complain.

The printf format specifier warning is not primarily a cross-platform portability checker. And, although in a few limited cases it can act somewhat like one, in general it does not. Take my previous example -- you get no warning on a platform that has int64_t as a typedef for long -- if this feature is to be useful as a portability checker, it should require that you used the PRId64 macro. Or, given ssize_t x = 0; printf("%ld", x);, it doesn't tell you to use "%zd" instead if ssize_t is a typedef for long -- although to be portable you ought to.

No, the major usefulness of the printf warning is to tell you that your code is incorrect for the _current_ platform. And most importantly when you chose the wrong size for your argument.

Those types which have matched size and alignment are still different types, and so it's technically appropriate to warn about using the wrong specifier there, too. But it's also entirely reasonable to not want to be bothered by such useless trivia, so skipping the warning when there's only a technical and not actual mismatch seems entirely sensible. (I might suggest that it should even be the default behavior for the warning, and if you want the stricter checks you'd ask for them...but I bet I'll get more pushback on that...)

Tue, May 8, 2:11 PM
smeenai added a comment to D46593: Allow copy elision in path concatenation.

Generating the patch from libc++ is fine (and this patch looks like it has sane paths).

Tue, May 8, 11:50 AM
smeenai added a reviewer for D46593: Allow copy elision in path concatenation: mclow.lists.
Tue, May 8, 11:36 AM
smeenai added a comment to D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers.
In D42933#1090268, @jfb wrote:

I was just looking at this, and I think @arphaman's patch is pretty much the right approach (with 2 suggested fixes below).

I don't think the code we're currently warning on is broken: a user code has NSInteger with %zd or NSUInteger with %zu, and on all platforms which support those types the implementor has guaranteed that (sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == sizeof(NSUInteger)).

Yes, but is this guaranteed to be the case or does it happen to be the case? I'm worried about the less mainstream uses where you might find ObjC code where this does not hold but -Wformat points out a portability concern.

Tue, May 8, 8:52 AM

Mon, May 7

smeenai added a comment to D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers.

Note that the alignment matters in addition to the size.

Mon, May 7, 1:22 PM
smeenai abandoned D42855: [private] Testing.

Done testing

Mon, May 7, 12:03 PM
smeenai added a comment to D46527: Object: Find terminator correctly when getting long filenames from GNU archives.

Is this fixing the same issue as D46214?

Mon, May 7, 8:57 AM

Sat, May 5

smeenai added a comment to D46482: [ObjCARC] Prevent code motion into a catchswitch.

No worries; thanks for all the other reviews, and for adding Michael as a reviewer to this one :)

Sat, May 5, 1:06 AM

Fri, May 4

smeenai created D46482: [ObjCARC] Prevent code motion into a catchswitch.
Fri, May 4, 4:39 PM
smeenai committed rL331548: [ObjCARC] Account for catchswitch in bitcast insertion.
[ObjCARC] Account for catchswitch in bitcast insertion
Fri, May 4, 12:06 PM
smeenai closed D46412: [ObjCARC] Account for catchswitch in bitcast insertion.
Fri, May 4, 12:06 PM

Thu, May 3

smeenai created D46412: [ObjCARC] Account for catchswitch in bitcast insertion.
Thu, May 3, 4:55 PM

Wed, May 2

smeenai committed rL331422: [ObjCARC] Convert an if to an early continue. NFC.
[ObjCARC] Convert an if to an early continue. NFC
Wed, May 2, 6:24 PM
smeenai closed D46371: [ObjCARC] Convert an if to an early continue. NFC.
Wed, May 2, 6:24 PM
smeenai created D46371: [ObjCARC] Convert an if to an early continue. NFC.
Wed, May 2, 4:54 PM

Tue, May 1

smeenai committed rC331306: [libclang] Fix the type of 'int (Foo);'.
[libclang] Fix the type of 'int (Foo);'
Tue, May 1, 1:48 PM
smeenai committed rL331306: [libclang] Fix the type of 'int (Foo);'.
[libclang] Fix the type of 'int (Foo);'
Tue, May 1, 1:48 PM
smeenai closed D45713: [libclang] Fix the type of 'int (Foo);'.
Tue, May 1, 1:48 PM
smeenai committed rL331305: [ARM] Remove redundant #if in test. NFC.
[ARM] Remove redundant #if in test. NFC
Tue, May 1, 1:41 PM
smeenai committed rC331305: [ARM] Remove redundant #if in test. NFC.
[ARM] Remove redundant #if in test. NFC
Tue, May 1, 1:41 PM
smeenai closed D45779: [ARM] Remove redundant #if in test.
Tue, May 1, 1:41 PM

Mon, Apr 30

smeenai accepted D46287: [Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets.

Makes sense to me. You may wanna wait for @zturner, but I can't really imagine any benefit to restricting this to MSVC.

Mon, Apr 30, 6:40 PM

Apr 26 2018

smeenai committed rLLD331000: [ELF] Clarify help wording for --symbol-ordering-file.
[ELF] Clarify help wording for --symbol-ordering-file
Apr 26 2018, 3:28 PM
smeenai committed rL331000: [ELF] Clarify help wording for --symbol-ordering-file.
[ELF] Clarify help wording for --symbol-ordering-file
Apr 26 2018, 3:28 PM
smeenai closed D46099: [ELF] Clarify help wording for --symbol-ordering-file.
Apr 26 2018, 3:28 PM
smeenai updated the diff for D46099: [ELF] Clarify help wording for --symbol-ordering-file.

Update with @grimar's suggestion

Apr 26 2018, 3:26 PM
smeenai added a comment to D46099: [ELF] Clarify help wording for --symbol-ordering-file.

The new message is IMO slightly better, so I am OK with that patch.

But I am actually thinking about something like:
"Layout sections to place symbols in the order specified by symbol ordering file"

Apr 26 2018, 2:51 PM

Apr 25 2018

smeenai committed rL330924: [cmake] Make linker detection take flags into account.
[cmake] Make linker detection take flags into account
Apr 25 2018, 11:08 PM
smeenai closed D45464: [cmake] Make linker detection take flags into account.
Apr 25 2018, 11:08 PM
smeenai added a comment to D46099: [ELF] Clarify help wording for --symbol-ordering-file.
In D46099#1078997, @pcc wrote:

One could also read "Lay out symbols" as meaning the order of symbols in the symbol table, so I'm not sure if this is better. No strong opinions though.

Apr 25 2018, 6:28 PM
smeenai created D46099: [ELF] Clarify help wording for --symbol-ordering-file.
Apr 25 2018, 6:10 PM
smeenai added a comment to D45464: [cmake] Make linker detection take flags into account.

(Gentle ping. This isn't urgent, but it would be nice to have.)

Apr 25 2018, 5:54 PM
smeenai added a reviewer for D45464: [cmake] Make linker detection take flags into account: phosek.
Apr 25 2018, 5:54 PM
smeenai updated the diff for D45464: [cmake] Make linker detection take flags into account.

Only read CMAKE_EXE_LINKER_FLAGS, to be consistent with CMake under CMP0056

Apr 25 2018, 5:52 PM
smeenai added a comment to D45639: [Driver] Support default libc++ library location on Darwin.

because the headers that are part of Clang toolchain are incompatible with the system library

Do you have details on this? This isn't supposed to be the case as far as I know. We link chrome/mac against system libc++ with the bundled headers, and it at least seems to work fine (and, from what I understand, is supposed to work as well).

Apr 25 2018, 11:45 AM

Apr 24 2018

smeenai committed rL330761: [cmake] Fix libc++ detection.
[cmake] Fix libc++ detection
Apr 24 2018, 12:50 PM

Apr 20 2018

smeenai committed rL330489: [ObjCARC] Take BlockColors by const reference. NFC.
[ObjCARC] Take BlockColors by const reference. NFC
Apr 20 2018, 3:18 PM
smeenai committed rL330487: [ObjCARC] Account for funclet token in storeStrong transform.
[ObjCARC] Account for funclet token in storeStrong transform
Apr 20 2018, 3:14 PM
smeenai closed D45857: [ObjCARC] Account for funclet token in storeStrong transform.
Apr 20 2018, 3:14 PM

Apr 19 2018

smeenai created D45857: [ObjCARC] Account for funclet token in storeStrong transform.
Apr 19 2018, 7:01 PM
smeenai added inline comments to D45801: COFF: Use (name, output characteristics) as a key when grouping input sections into output sections..
Apr 19 2018, 3:30 PM
smeenai added inline comments to D45801: COFF: Use (name, output characteristics) as a key when grouping input sections into output sections..
Apr 19 2018, 1:15 PM
smeenai accepted D45829: Remove impossible _MSC_VER check.

LGTM. I'd tried to clean all stale _MSC_VER conditionals in the past, but I must have missed some.

Apr 19 2018, 12:38 PM
smeenai added a comment to D45832: [PDB] Output first section contribution for each module.

Typo in the summary: refernece -> reference

Apr 19 2018, 12:37 PM

Apr 18 2018

smeenai accepted D45779: [ARM] Remove redundant #if in test.

LGTM; it's an obvious NFC patch.

Apr 18 2018, 11:40 AM

Apr 12 2018

smeenai accepted D45529: [CMake] Set the default ABI version for Fuchsia in CMake as well.

Eric accepted on IRC.

Apr 12 2018, 5:23 PM
smeenai added reviewers for D45601: Warn on bool* to bool conversion: lebedev.ri, rjmccall.
Apr 12 2018, 4:38 PM

Apr 11 2018

smeenai added a comment to D45529: [CMake] Set the default ABI version for Fuchsia in CMake as well.

This LGTM given that Eric accepted your previous change. I'm happy to accept it in a day or two if he hasn't gotten the chance to take a look by then.

Apr 11 2018, 9:49 PM
smeenai committed rC329836: [CodeGen] Handle __func__ inside __finally.
[CodeGen] Handle __func__ inside __finally
Apr 11 2018, 11:20 AM
smeenai committed rL329836: [CodeGen] Handle __func__ inside __finally.
[CodeGen] Handle __func__ inside __finally
Apr 11 2018, 11:20 AM
smeenai closed D45523: [CodeGen] Handle __func__ inside __finally.
Apr 11 2018, 11:20 AM
smeenai added inline comments to D45523: [CodeGen] Handle __func__ inside __finally.
Apr 11 2018, 8:56 AM
smeenai updated the summary of D45523: [CodeGen] Handle __func__ inside __finally.
Apr 11 2018, 8:53 AM
smeenai created D45523: [CodeGen] Handle __func__ inside __finally.
Apr 11 2018, 8:52 AM

Apr 9 2018

smeenai added a comment to D45467: COFF: Friendlier undefined symbol errors..

I just noticed you said on non-Windows. Yea, I think the path should be whatever is native to the host you're building on.

Apr 9 2018, 6:47 PM
smeenai created D45464: [cmake] Make linker detection take flags into account.
Apr 9 2018, 3:38 PM

Apr 6 2018

smeenai added a comment to D45399: Split SectionPiece in its parts.

Patch is missing context.

Apr 6 2018, 9:27 PM

Apr 4 2018

smeenai added a comment to D45261: [ELF] - Allow LLD to produce file symbols..

@ruiu just in case you didn't know, the URL https://llvm.org/PR36716 will do a redirect to the right place.

Apr 4 2018, 12:21 PM

Apr 3 2018

smeenai planned changes to D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall.

This doesn't pass all tests right now, and I'll also need to change the test in accordance with the review comments.

Apr 3 2018, 8:42 PM
smeenai added a comment to D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall.

Finally got a chance to dig into some of the failures this patch is causing. Here's an example crasher (run with clang -cc1 -triple i686-windows -emit-llvm):

Apr 3 2018, 8:41 PM

Mar 27 2018

smeenai committed rL328654: [Sema] Avoid crash for category implementation without interface.
[Sema] Avoid crash for category implementation without interface
Mar 27 2018, 12:02 PM
smeenai committed rC328654: [Sema] Avoid crash for category implementation without interface.
[Sema] Avoid crash for category implementation without interface
Mar 27 2018, 12:02 PM
smeenai closed D44916: [Sema] Avoid crash for category implementation without interface.
Mar 27 2018, 12:02 PM

Mar 26 2018

smeenai created D44916: [Sema] Avoid crash for category implementation without interface.
Mar 26 2018, 7:25 PM

Mar 22 2018

smeenai added a comment to D44778: [clang-format] Wildcard expansion on Windows..

PURE_WINDOWS is all Windows except Cygwin. It's an LLVM thing, not a CMake thing.

Mar 22 2018, 9:47 AM

Mar 20 2018

smeenai added a comment to D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall.

Thanks Reid. I still need to look into why this is causing some existing tests to crash, but I'll also adjust the test.

Mar 20 2018, 3:30 PM
smeenai committed rL328042: [ObjCARC] Add funclet token to ARC marker.
[ObjCARC] Add funclet token to ARC marker
Mar 20 2018, 1:48 PM
smeenai closed D44641: [ObjCARC] Add funclet token to ARC marker.
Mar 20 2018, 1:48 PM
smeenai added a comment to D27296: Don't assume mingw is providing SSP functions.

Seems fine to me, since windows-itanium is unaffected.

Mar 20 2018, 12:13 PM

Mar 19 2018

smeenai added a comment to D44671: [libcxx] Enable static libcxxabi linking on Darwin.

I would imagine there was a reason this configuration was unsupported on macOS – perhaps something to do with libc++/libc++abi being the default system libraries on that platform?

Mar 19 2018, 10:04 PM
smeenai added a comment to D44625: Accept any filepath in llvm_check_source_file_list.

Sweet, thanks for the update.

Mar 19 2018, 2:34 PM
smeenai added inline comments to D44625: Accept any filepath in llvm_check_source_file_list.
Mar 19 2018, 1:47 PM
smeenai added a reviewer for D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics: compnerd.

@DHowett-MSFT the reviewers look fine to me. Reid is the code owner for clang's MSVC compat support. David doesn't work on this stuff directly anymore, I think, but he's still pretty active in code reviews for it. I'm adding Saleem, who's also pretty active on Windows stuff.

Mar 19 2018, 1:13 PM · Restricted Project
smeenai committed rL327892: [CodeGen] Add funclet token to ARC marker.
[CodeGen] Add funclet token to ARC marker
Mar 19 2018, 12:38 PM
smeenai committed rC327892: [CodeGen] Add funclet token to ARC marker.
[CodeGen] Add funclet token to ARC marker
Mar 19 2018, 12:38 PM
smeenai closed D44640: [CodeGen] Add funclet token to ARC marker.
Mar 19 2018, 12:38 PM
smeenai closed D44640: [CodeGen] Add funclet token to ARC marker.
Mar 19 2018, 12:38 PM
smeenai accepted D44625: Accept any filepath in llvm_check_source_file_list.

LGTM with the comments addressed.

Mar 19 2018, 12:09 PM
smeenai created D44641: [ObjCARC] Add funclet token to ARC marker.
Mar 19 2018, 12:01 PM
smeenai created D44640: [CodeGen] Add funclet token to ARC marker.
Mar 19 2018, 11:57 AM
smeenai added a reviewer for D44625: Accept any filepath in llvm_check_source_file_list: beanz.

Could you upload the patch with context (-U9999) please?

Mar 19 2018, 8:41 AM