rsmith (Richard Smith)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 12 2012, 2:19 PM (249 w, 4 d)

Recent Activity

Yesterday

rsmith committed rL301285: Placate MSVC's narrowing conversion unhappiness..
Placate MSVC's narrowing conversion unhappiness.
Mon, Apr 24, 8:41 PM
rsmith accepted D28832: Improve redefinition errors pointing to the same header..

LGTM. I'd like to make sure we try to use something better than the first import location for a module (that location is especially meaningless under -fmodules-local-submodule-visibility), but I'm happy for that (the big comment) to be dealt with as a follow-on change, and all the other comments here are minor, so feel free to commit after addressing those.

Mon, Apr 24, 5:46 PM
rsmith committed rL301271: [modules ts] Diagnose 'export' declarations outside of a module interface..
[modules ts] Diagnose 'export' declarations outside of a module interface.
Mon, Apr 24, 4:25 PM

Fri, Apr 21

rsmith committed rL301066: Rearrange some Modules TS testcases into test/CXX/modules-ts..
Rearrange some Modules TS testcases into test/CXX/modules-ts.
Fri, Apr 21, 6:00 PM
rsmith committed rL301056: P0629R0: Switch to latest proposal for distinguishing module interface from….
P0629R0: Switch to latest proposal for distinguishing module interface from…
Fri, Apr 21, 3:52 PM
rsmith added inline comments to D32378: Insert invariant.group.barrier for pointers comparisons.
Fri, Apr 21, 3:41 PM
rsmith added a comment to D31856: Headers: Make the type of SIZE_MAX the same as size_t.

We normally use stdint.h from the host C library, rather than our own version; this file is only relevant in -ffreestanding mode. So it should be safe to change.

Fri, Apr 21, 2:46 PM
rsmith added inline comments to D32309: [libcxx] [test] Resolve compiler warnings in LCM/GCD tests.
Fri, Apr 21, 2:35 PM
rsmith added a comment to D32372: Arrays of unknown bound in constant expressions.

This needs testcases (the one from your summary plus the ones in my comments above would be good).

Fri, Apr 21, 2:30 PM

Thu, Apr 20

rsmith committed rL300938: [modules] Properly look up the owning module for an instantiation of a merged….
[modules] Properly look up the owning module for an instantiation of a merged…
Thu, Apr 20, 6:28 PM
rsmith added a comment to D32269: [Driver] Add iSOFTLinux to GNU ToolChains X86Triple.

Please add some test coverage for these triples.

Thu, Apr 20, 4:28 PM
rsmith requested changes to D31856: Headers: Make the type of SIZE_MAX the same as size_t.

This is sadly not a correct change. The relevant requirements (C11 7.20.3/2) on these macros are:

Thu, Apr 20, 4:26 PM
rsmith accepted D32322: Use __CLANG_ATOMIC_TYPE_LOCK_FREE macros in `stdatomic.h`.
Thu, Apr 20, 4:17 PM
rsmith accepted D32265: Add __CLANG_ATOMIC_<TYPE>_LOCK_FREE macros for use in MSVC compatibility mode..

I'm not thrilled about adding yet more predefined macros, but it really doesn't make sense for libc++ to depend on __GCC_* macros when targeting Windows, nor for these to be Windows-only, so let's do it.

Thu, Apr 20, 3:35 PM
rsmith added a comment to D32199: [TySan] A Type Sanitizer (Clang).
  1. C's "effective type" rule allows writes to set the type pretty much unconditionally, unless the storage is for a variable with a declared type

To come back to this point: We don't really implement these rules now, and it is not clear that we will. The problem here is that, if we take the specification literally, then we can't use our current TBAA at all. The problem is that if we have:

write x, !tbaa "int"
read x, !tbaa "int"
write x, !tbaa "float"

TBAA will currently tell us that the "float" write aliases with neither the preceding read nor the preceding write.

Thu, Apr 20, 3:02 PM

Wed, Apr 19

rsmith committed rL300805: PR32673: Don't wrap parameter packs in SubstTemplateTypeParmPackType nodes when….
PR32673: Don't wrap parameter packs in SubstTemplateTypeParmPackType nodes when…
Wed, Apr 19, 6:28 PM
rsmith added a comment to D32199: [TySan] A Type Sanitizer (Clang).

As I've currently implemented it, both reads and writes set the type of previously-unknown storage, and after that it says fixed (unless you memcpy to it, memset it, or its lifetime ends (the type gets reset on lifetime.start/end and for malloc/allocas/etc.). There's a flag to enable the "writes always set the type" rule, but that's not the default. Is this too strict?

Wed, Apr 19, 6:24 PM
rsmith committed rL300803: Add a triple to codegen test..
Add a triple to codegen test.
Wed, Apr 19, 6:15 PM
rsmith added inline comments to D32251: Implement DR1601 - Promotion of enumeration with fixed underlying type.
Wed, Apr 19, 5:53 PM
rsmith added inline comments to D31778: [Modules] Implement ODR-like semantics for tag types in C/ObjC.
Wed, Apr 19, 4:56 PM
rsmith added a comment to D32199: [TySan] A Type Sanitizer (Clang).

! In D32199#731252, @hfinkel wrote:

How about renaming this to something more like -fsanitize=type?

I'm fine with that. Do you like TypeSanitizer or TypeAccessSantizer or TypeAliasingSanitizer best?

Wed, Apr 19, 4:37 PM
rsmith added a comment to D32199: [TySan] A Type Sanitizer (Clang).

I don't like calling this a "TBAA sanitizer". What we're sanitizing is the object model and effective type rules; it seems irrelevant which specific compiler analysis passes would result in your program misbehaving if you break the rules. I would also expect that we will extend this in future to assign types to storage even in cases where there is no store (for instance, we should be able to catch float f() { int n; return *(float*)&n; } despite there being no TBAA violation in the naive IR).

Wed, Apr 19, 3:03 PM
rsmith committed rL300762: Fix assertion failure in codegen on non-template deduction guide..
Fix assertion failure in codegen on non-template deduction guide.
Wed, Apr 19, 2:28 PM
rsmith committed rL300728: Update comment to match r300252..
Update comment to match r300252.
Wed, Apr 19, 11:30 AM

Tue, Apr 18

rsmith committed rL300653: Fix member function call with null 'this' pointer..
Fix member function call with null 'this' pointer.
Tue, Apr 18, 7:32 PM
rsmith updated the summary of D32211: Collapse AArch64/Utils into AArch64/MCTargetDesc.
Tue, Apr 18, 7:15 PM
rsmith created D32211: Collapse AArch64/Utils into AArch64/MCTargetDesc.
Tue, Apr 18, 7:15 PM
rsmith committed rL300650: [modules] Properly look up the owning module for an instantiation of a merged….
[modules] Properly look up the owning module for an instantiation of a merged…
Tue, Apr 18, 6:49 PM
rsmith committed rL300611: [modules-ts] Fold together -x c++ and -x c++-module at -cc1 level..
[modules-ts] Fold together -x c++ and -x c++-module at -cc1 level.
Tue, Apr 18, 3:08 PM
rsmith committed rL300609: Do not warn about whitespace between ??/ trigraph and newline in line comments….
Do not warn about whitespace between ??/ trigraph and newline in line comments…
Tue, Apr 18, 2:58 PM

Mon, Apr 17

rsmith committed rL300515: Fix mishandling of escaped newlines followed by newlines or nuls..
Fix mishandling of escaped newlines followed by newlines or nuls.
Mon, Apr 17, 4:57 PM

Fri, Apr 14

rsmith added a comment to D32092: Attribute inline.

From some very superficial testing, it looks like CL treats __declspec(inline) exactly like a synonym for inline (it even rejects them both appearing on the same declaration, complaining about a duplicate inline specifier). So modeling this as GNUInlineAttr is definitely wrong, and this should probably be setting the inline flag on the function. But I agree with Aaron: we need some evidence that implementing this in Clang is worthwhile (rather than changing the codebase to use the inline keyword instead). We aim to be CL-compatible for common code patterns, not to be 100% compatible with all code that CL accepts.

Fri, Apr 14, 2:03 PM
rsmith added a comment to D32092: Attribute inline.

Pushed the submit too fast ...
Before I submitted this review, I have done some experiments and inline and declspec(inline) have the same behavior. Compiling with /Ob0 disables inlining. With -O1 or -O2, inline happens.

Fri, Apr 14, 1:13 PM
rsmith added inline comments to D32092: Attribute inline.
Fri, Apr 14, 12:55 PM

Thu, Apr 13

rsmith committed rL300296: Remove empty test directory for nonexistent standard clause..
Remove empty test directory for nonexistent standard clause.
Thu, Apr 13, 7:17 PM
rsmith committed rL300271: [docs] Regenerate diagnostics reference..
[docs] Regenerate diagnostics reference.
Thu, Apr 13, 3:57 PM
rsmith committed rL300270: [docs] Fix a couple of typos in command line flag help text and regenerate….
[docs] Fix a couple of typos in command line flag help text and regenerate…
Thu, Apr 13, 3:52 PM
rsmith added a comment to D27263: Address of bitfield in anonymous struct doesn't error..

The change to test/SemaCXX/anonymous-struct.cpp appeared to be unrelated to the rest of the patch, so I committed it separately as r300266.

Thu, Apr 13, 3:04 PM
rsmith committed rL300266: Add test for anonymous struct containing an implicitly private data member..
Add test for anonymous struct containing an implicitly private data member.
Thu, Apr 13, 3:03 PM
rsmith committed rL300264: Diagnose attempt to take address of bitfield members in anonymous structs..
Diagnose attempt to take address of bitfield members in anonymous structs.
Thu, Apr 13, 3:02 PM
rsmith closed D27263: Address of bitfield in anonymous struct doesn't error. by committing rL300264: Diagnose attempt to take address of bitfield members in anonymous structs..
Thu, Apr 13, 3:02 PM
rsmith committed rL300262: PR32185: Revert r291512 and add a testcase for PR32185..
PR32185: Revert r291512 and add a testcase for PR32185.
Thu, Apr 13, 2:50 PM
rsmith added a comment to D31968: Remove all allocation and divisions from GreatestCommonDivisor.

Accidentally committed a couple of extra files, reverted in r300253.

Thu, Apr 13, 1:44 PM
rsmith committed rL300253: Revert accidentally-committed files in r300252..
Revert accidentally-committed files in r300252.
Thu, Apr 13, 1:44 PM
rsmith committed rL300252: Remove all allocation and divisions from GreatestCommonDivisor.
Remove all allocation and divisions from GreatestCommonDivisor
Thu, Apr 13, 1:42 PM
rsmith closed D31968: Remove all allocation and divisions from GreatestCommonDivisor by committing rL300252: Remove all allocation and divisions from GreatestCommonDivisor.
Thu, Apr 13, 1:42 PM
rsmith added inline comments to D31968: Remove all allocation and divisions from GreatestCommonDivisor.
Thu, Apr 13, 1:41 PM
rsmith added inline comments to D27546: [ASTReader] Sort RawComments before merging.
Thu, Apr 13, 1:17 PM

Wed, Apr 12

rsmith committed rL300151: Fix broken test. We can't assume that 2MB of args is enough to require a….
Fix broken test. We can't assume that 2MB of args is enough to require a…
Wed, Apr 12, 5:59 PM
rsmith committed rL300144: Work around MSVC rejects-valid bug related to C++11 narrowing conversions..
Work around MSVC rejects-valid bug related to C++11 narrowing conversions.
Wed, Apr 12, 5:27 PM
rsmith committed rL300141: Update to match LLVM r300135..
Update to match LLVM r300135.
Wed, Apr 12, 5:03 PM
rsmith committed rL300139: Fix some ArgList uses after API change in r300135..
Fix some ArgList uses after API change in r300135.
Wed, Apr 12, 4:56 PM
rsmith committed rL300136: Update to match LLVM r300135..
Update to match LLVM r300135.
Wed, Apr 12, 4:34 PM
rsmith committed rL300135: ArgList: cache index ranges containing arguments with each ID.
ArgList: cache index ranges containing arguments with each ID
Wed, Apr 12, 4:32 PM
rsmith closed D30130: ArgList: cache index ranges containing arguments with each ID by committing rL300135: ArgList: cache index ranges containing arguments with each ID.
Wed, Apr 12, 4:32 PM
rsmith added a comment to D31968: Remove all allocation and divisions from GreatestCommonDivisor.

LGTM with the one question in the test cases.

Wed, Apr 12, 4:30 PM
rsmith updated the diff for D31968: Remove all allocation and divisions from GreatestCommonDivisor.
Wed, Apr 12, 2:41 PM
rsmith added a comment to D31968: Remove all allocation and divisions from GreatestCommonDivisor.

Can we just merge lshrNear fully into lshrInPlace and just use lshrInPlace in the other place that uses lshrNear?

Wed, Apr 12, 2:40 PM

Tue, Apr 11

rsmith created D31968: Remove all allocation and divisions from GreatestCommonDivisor.
Tue, Apr 11, 6:17 PM
rsmith accepted D27604: [Driver] Add compiler option to generate a reproducer.

LGTM with one change.

Tue, Apr 11, 11:59 AM
rsmith accepted D31781: [Modules] Allow local submodule visibility without c++.

Can you also add a basic test that this works in C? Thanks!

Tue, Apr 11, 11:37 AM

Mon, Apr 10

rsmith accepted D29877: Warn about unused static file scope function template declarations..

LGTM with the overloaded operator check removed.

Mon, Apr 10, 2:19 PM
rsmith added inline comments to D29877: Warn about unused static file scope function template declarations..
Mon, Apr 10, 2:19 PM
rsmith accepted D30793: [modules] Delay calling DeclMustBeEmitted until it's safe.

This is not a complete fix, since the call within loadDeclUpdateRecords could still be problematic, but it's at least an improvement.

Mon, Apr 10, 2:09 PM

Mar 23 2017

rsmith committed rL298676: Fix handling of initialization from parenthesized initializer list..
Fix handling of initialization from parenthesized initializer list.
Mar 23 2017, 6:26 PM
rsmith committed rL298663: Remove uses of std::binary_function, removed in C++17..
Remove uses of std::binary_function, removed in C++17.
Mar 23 2017, 4:44 PM
rsmith committed rL298657: Remove all uses of std::mem_fun and std::bind1st removed in C++17..
Remove all uses of std::mem_fun and std::bind1st removed in C++17.
Mar 23 2017, 4:30 PM
rsmith committed rL298645: Remove unnecessary use of std::result_of, which is deprecated in C++17..
Remove unnecessary use of std::result_of, which is deprecated in C++17.
Mar 23 2017, 2:14 PM

Mar 22 2017

rsmith accepted D31187: Fix removal of out-of-line definitions..

The patch itself LGTM. I'd like some test coverage, but if this will be covered by some upcoming interpreter piece and you need this to unblock yourselves, that's good enough for me. In any case, adding a full-blown UnitTest check for this seems like overkill...

Mar 22 2017, 5:17 PM
rsmith accepted D31190: Publish RAIIObjectsForParser.h for external usage.
Mar 22 2017, 3:53 PM
rsmith added inline comments to D30793: [modules] Delay calling DeclMustBeEmitted until it's safe.
Mar 22 2017, 3:41 PM

Mar 21 2017

rsmith committed rL298477: Suppress warning on unreachable [[clang::fallthrough]] within a template….
Suppress warning on unreachable [[clang::fallthrough]] within a template…
Mar 21 2017, 7:01 PM
rsmith closed D31069: Don't warn about an unreachable fallthrough annotation in a template function by committing rL298477: Suppress warning on unreachable [[clang::fallthrough]] within a template….
Mar 21 2017, 7:01 PM
rsmith accepted D30954: Modules: Simulate diagnostic settings for implicit modules.
  • The patch-as-is checks whether pragmas should be demoted to warnings for all AST files, not just implicit modules. I can add a bit of logic to ReadPragmaDiagnosticMappings that limits it to F.Kind == MK_ImplicitModule, but I'm not sure it's necessary. Maybe it hits when reading PCH files, but no tests fail, and I'm not sure this is worse... thoughts?
Mar 21 2017, 4:12 PM

Mar 20 2017

rsmith committed rL298300: Avoid these headers looking like the same file on a content-addressed file….
Avoid these headers looking like the same file on a content-addressed file…
Mar 20 2017, 1:26 PM
rsmith committed rL298299: Bump __cplusplus for C++17 to 201703L per the C++17 DIS..
Bump __cplusplus for C++17 to 201703L per the C++17 DIS.
Mar 20 2017, 1:25 PM

Mar 17 2017

rsmith accepted D30848: Implement DR 373 "Lookup on namespace qualified name in using-directive".

Thanks, LGTM

Mar 17 2017, 9:12 AM

Mar 16 2017

rsmith accepted D31069: Don't warn about an unreachable fallthrough annotation in a template function.

LGTM, do you need someone to commit for you?

Mar 16 2017, 10:36 PM
rsmith added a comment to D31069: Don't warn about an unreachable fallthrough annotation in a template function.

This needs a test case, but the change itself looks fine to me.

Mar 16 2017, 9:12 PM
rsmith added a reviewer for D31069: Don't warn about an unreachable fallthrough annotation in a template function: rsmith.
Mar 16 2017, 9:11 PM

Mar 13 2017

rsmith added inline comments to D29877: Warn about unused static file scope function template declarations..
Mar 13 2017, 1:56 PM

Mar 11 2017

rsmith added a comment to D30848: Implement DR 373 "Lookup on namespace qualified name in using-directive".

Functionally, this looks good. How do the diagnostics look in the case where lookup only finds a non-namespace name? Eg,

Mar 11 2017, 11:19 AM

Mar 10 2017

rsmith added inline comments to D30848: Implement DR 373 "Lookup on namespace qualified name in using-directive".
Mar 10 2017, 2:38 PM

Mar 9 2017

rsmith added a comment to D30793: [modules] Delay calling DeclMustBeEmitted until it's safe.

Can you also add the test from PR32186?

Mar 9 2017, 3:27 PM
rsmith committed rL297412: Add -cc1 flag -ast-dump-all to perform an AST dump including entities that….
Add -cc1 flag -ast-dump-all to perform an AST dump including entities that…
Mar 9 2017, 2:12 PM

Mar 8 2017

rsmith committed rL297349: Fix handling of -fmodule-map-file=X where X has no directory component..
Fix handling of -fmodule-map-file=X where X has no directory component.
Mar 8 2017, 5:20 PM
rsmith committed rL297316: Take into account C++17's noexcept function types during merging -- it should.
Take into account C++17's noexcept function types during merging -- it should
Mar 8 2017, 3:12 PM

Mar 7 2017

rsmith accepted D30711: [coroutines] update coro_end builtin to match llvm.
Mar 7 2017, 11:23 AM

Mar 6 2017

rsmith added inline comments to D26057: [coroutines] Add DependentCoawaitExpr and fix re-building CoroutineBodyStmt..
Mar 6 2017, 3:48 PM
rsmith accepted D26057: [coroutines] Add DependentCoawaitExpr and fix re-building CoroutineBodyStmt..
Mar 6 2017, 2:18 PM
rsmith accepted D30590: Don't assume cleanup emission preserves dominance in expr evaluation.

Hal and I discussed exactly the same problem (in the context of coroutines) on Saturday and came up with exactly the same approach :) I think this is the right direction.

Mar 6 2017, 1:09 PM
rsmith updated subscribers of D30590: Don't assume cleanup emission preserves dominance in expr evaluation.
Mar 6 2017, 1:00 PM

Feb 28 2017

rsmith added a comment to D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable.

It seems to me that the problem here is that DeclMustBeEmitted is not safe to call in the middle of deserialization if anything partially-deserialized might be reachable from the declaration we're querying, and yet we're currently calling it in that case.

DeclMustBeEmitted name seems misleading since we does more than just checking, it actually tries to emit stuff.

Feb 28 2017, 10:54 PM
rsmith accepted D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable.

Yes, I'm OK with this as a stopgap fix for 4.0. I would have preferred a more full fix for 4.0 but we've run out of time for that, and the PCH case does seem more pressing.

Feb 28 2017, 10:34 PM

Feb 27 2017

rsmith accepted D29979: [coroutines] Add co_return statement emission.
Feb 27 2017, 12:35 PM
rsmith accepted D27455: UBSan docs: Explicitly mention that `-fsanitize=unsigned-integer-overflow` does not catch UB..
Feb 27 2017, 11:48 AM

Feb 25 2017

rsmith committed rL296276: Update cxx_dr_status page..
Update cxx_dr_status page.
Feb 25 2017, 4:06 PM
rsmith committed rL296275: C++ DR1611, 1658, 2180: implement "potentially constructed subobject" rules for….
C++ DR1611, 1658, 2180: implement "potentially constructed subobject" rules for…
Feb 25 2017, 4:05 PM

Feb 24 2017

rsmith committed rL296173: Factor out more commonality between handling of deletion and exception….
Factor out more commonality between handling of deletion and exception…
Feb 24 2017, 1:30 PM
rsmith accepted D21626: Lit C++11 Compatibility Patch #10.

LGTM with a couple of changes.

Feb 24 2017, 1:23 PM