Page MenuHomePhabricator

doug.gregor (Doug Gregor)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 17 2012, 3:26 PM (357 w, 1 d)

Recent Activity

Jan 23 2019

doug.gregor accepted D57076: [ObjC generics] Fix applying `__kindof` to the type parameter..

One minor request about regarding a dyn_cast, but this looks like the right approach. Thank you!

Jan 23 2019, 9:56 PM · Restricted Project

Apr 18 2018

doug.gregor accepted D44670: [CXX] Templates specialization visibility can be wrong.

LGTM, thanks

Apr 18 2018, 10:24 PM

Apr 17 2018

doug.gregor added inline comments to D44670: [CXX] Templates specialization visibility can be wrong.
Apr 17 2018, 3:44 PM

Feb 13 2018

doug.gregor accepted D43239: Improve internal StringSwitch code gen with Optional<T>.

Heh, I was probably thinking that one might want to produce references, but I don’t anyone does. I do think it would be valuable to make the parameters “T value” instead of “const T value”, and std::move them into place when we find a match.

Feb 13 2018, 12:06 PM

Oct 24 2017

doug.gregor added a comment to D36790: [ObjC] Messages to 'self' in class methods that return 'instancetype' should use the pointer to the class as the result type of the message.

Yes, I think it's reasonable to treat instancetype as an inherited requirement. I guess the only exception would be if we had some notion of final classes or methods in Objective-C, in which case they'd be able to return a concrete type.

Oct 24 2017, 9:49 AM

Sep 14 2017

doug.gregor accepted D37089: [Sema] Error out early for tags defined inside an enumeration..

Looks good to me; sorry for the delay.

Sep 14 2017, 3:19 PM

Aug 10 2017

doug.gregor added a comment to D36530: [Parse] Document PrintStats, SkipFunctionBodies.

LGTM, thank you!

Aug 10 2017, 8:53 AM

Jul 26 2017

doug.gregor added a comment to D35923: Fix PR32332 - PCM nondeterminism with builtins caused by global module index .

I am *very* concerned about this. The optimization that avoids doing lookups within module files where we "know" we won't find anything was very important at the time it was introduced, so I'd like to understand better why it didn't make a significant difference in your benchmarks before we disable it. (Especially because of built-ins; those are Very Weird and probably modeled wrong).

Jul 26 2017, 4:56 PM

Jul 5 2017

doug.gregor closed D34996: Customize the SFINAE diagnostics for enable_if to provide the failed condition..

Committed as r307196 and r307197

Jul 5 2017, 1:22 PM · Restricted Project
doug.gregor added a comment to D34996: Customize the SFINAE diagnostics for enable_if to provide the failed condition..

Yes, digging into a static_assert to determine which condition caused it to fail would be quite useful QoI, and it should be fairly easy to use this mechanism to do that. We might want to dig one level deeper to give more information, e.g., static_assert(Comparable<T>) could look into the definition of Comparable<T> to see which piece of it failed.

Jul 5 2017, 9:29 AM · Restricted Project
doug.gregor added inline comments to D34996: Customize the SFINAE diagnostics for enable_if to provide the failed condition..
Jul 5 2017, 9:16 AM · Restricted Project

Jul 4 2017

doug.gregor created D34996: Customize the SFINAE diagnostics for enable_if to provide the failed condition..
Jul 4 2017, 11:26 PM · Restricted Project

Jun 28 2017

doug.gregor accepted D34741: [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces..

Looks great!

Jun 28 2017, 10:12 AM

Mar 29 2017

doug.gregor accepted D31378: [PCH] Attach instance's dependency collectors to PCH external AST sources..

LGTM!

Mar 29 2017, 9:50 AM

Jan 5 2017

doug.gregor added a comment to D28349: [Frontend] The macro that describes the Objective-C bool type should be defined in C as well.

The "!LangOpts.CPlusPlus" doesn't make sense to me. You're presumably defining this macro in C because the Objective-C runtime is usable from C, but that same logic applies to C++ code. It seems like we should be defining this macro unconditionally.

Jan 5 2017, 8:54 AM

Dec 8 2016

doug.gregor accepted D27580: [modules] Add optional out-param to ASTReader::ReadAST for imported submodules..

LGTM!

Dec 8 2016, 10:21 AM

Nov 10 2016

doug.gregor accepted rL286542: Don't require nullability on 'va_list', even when it's a pointer..

Yeah, LGTM.

Nov 10 2016, 9:23 PM

Nov 9 2016

doug.gregor accepted D26227: Don't require nullability on 'va_list'..

I don't see a better way to do this. Nice meta programming hack.

Nov 9 2016, 1:17 PM
doug.gregor accepted D26109: Warn when 'assume_nonnull' infers nullability within an array..

LGTM!

Nov 9 2016, 1:15 PM
doug.gregor accepted D26108: Add -Wnullability-completeness-on-arrays..

Looks good! I appreciate the refactoring of recordNullabilitySeen

Nov 9 2016, 1:07 PM
doug.gregor accepted D25850: Accept nullability annotations (_Nullable) on array parameters.

This is looking great, Jordan.

Nov 9 2016, 11:49 AM

Nov 1 2016

doug.gregor accepted D26226: Don't require nullability on template parameters in typedefs..

Seem fine; I'd rename the "FIXME" to a "Note" unless you're going to pass a flag to Type::canHaveNullability to say how to classify dependent types.

Nov 1 2016, 10:57 PM

Oct 19 2016

doug.gregor added a comment to D24902: A couple small inconsequential fixes.

Do any of the regression tests make use of this diagnostic? They might have to change their "expected-error" checks to look for the corrected wording.

Oct 19 2016, 9:39 AM

Sep 12 2016

doug.gregor accepted D24059: NFC: refactor applyObjCProtocolQualifiers from SemaType.cpp to ASTContext so it can be shared..

Yep, this refactor looks good!

Sep 12 2016, 4:07 PM
doug.gregor accepted D23080: ObjC: Use a new type for ObjC type parameter (patch 3 out of 3).

Ahhh, much cleaner. Thanks!

Sep 12 2016, 4:06 PM
doug.gregor accepted D23079: ObjC: Use a new type for ObjC type parameter (patch 2 out of 3).

This looks great, thank you!

Sep 12 2016, 4:05 PM
doug.gregor accepted D23852: [SemaObjC] Fix crash while parsing type arguments and protocols.

I don't love the fact that it makes callers fragile, but having to do tentative parsing for these otherwise-unambiguous cases is expensive (in compile time). We should only use tentative parsing when we have an actual ambiguity to resolve.

Sep 12 2016, 3:49 PM

Aug 25 2016

doug.gregor added a comment to D23852: [SemaObjC] Fix crash while parsing type arguments and protocols.

This will work, but it's *really* unfortunate to put tentative parsing into this code path because tentative parsing is far from free, and specialized Objective-C types are getting pretty common in Objective-C headers. Why can't the callers be taught to handle EOF and bail out?

Aug 25 2016, 9:16 AM

Aug 23 2016

doug.gregor added inline comments to D23079: ObjC: Use a new type for ObjC type parameter (patch 2 out of 3).
Aug 23 2016, 3:46 PM

Aug 19 2016

doug.gregor requested changes to D23080: ObjC: Use a new type for ObjC type parameter (patch 3 out of 3).

I think most of the complexity here will fold away when ObjCTypeParamType becomes sugar for the underlying ObjCObjectPointerType.

Aug 19 2016, 4:39 PM
doug.gregor requested changes to D23079: ObjC: Use a new type for ObjC type parameter (patch 2 out of 3).

A couple of comments above, but this is looking very good.

Aug 19 2016, 3:50 PM
doug.gregor accepted D23078: ObjC: Use a new type for ObjC type parameter (Patch 1 out of 3).

This refactor LGTM.

Aug 19 2016, 3:31 PM

Jul 19 2016

doug.gregor accepted D22392: [Sema] Fix an invalid nullability warning for binary conditional operators.

LGTM!

Jul 19 2016, 3:50 PM

Jul 14 2016

doug.gregor accepted D22248: [Sema] Create a separate group for incompatible function pointer warning.

LGTM!

Jul 14 2016, 9:39 PM
doug.gregor added a comment to D22392: [Sema] Fix an invalid nullability warning for binary conditional operators.

A bunch of comments above. This needs much more extensive testing, because there are numerous paths through the ternary operator code and the results need to be symmetric.

Jul 14 2016, 9:26 PM
doug.gregor added a comment to D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type .

I think this check should go into SemaChecking.cpp

Jul 14 2016, 7:55 PM

May 31 2016

doug.gregor accepted D20383: PCH + Module: make sure we write out macros associated with builtin identifiers.

Yes, that's a LGTM, sorry for being unclear.

May 31 2016, 10:54 AM

May 30 2016

doug.gregor added a comment to D20383: PCH + Module: make sure we write out macros associated with builtin identifiers.

Yeah, this looks like the right approach. PCH follows the same rules as modules when it comes to newer information shadowing imported information.

May 30 2016, 8:16 AM

May 27 2016

doug.gregor accepted D20451: [Parser] Fix look ahead after EOF while parsing objc message and lambdas.

LGTM, sorry for the delay!

May 27 2016, 8:46 PM

Apr 29 2016

doug.gregor accepted D19679: Method pool in modules: sync up out of date selectors before writing the module.

LGTM!

Apr 29 2016, 11:43 AM
doug.gregor added a comment to D19679: Method pool in modules: sync up out of date selectors before writing the module.

Mostly looks good, but see my comment about MapVector invalidation.

Apr 29 2016, 11:15 AM

Apr 25 2016

doug.gregor accepted D19443: Module Debugging: Fix the condition for determining whether a template instantiation is in a module..

Yeah, this LGTM.

Apr 25 2016, 10:03 AM

Apr 11 2016

doug.gregor added a comment to D18823: Implementation of VlA of GNU C++ extension.

I think it's completely reasonable to implement support for VLAs as a GNU C++ extension. We did go through a phase where we tried to avoid implementing VLAs in C++ because we considered them to be a poor feature in C++. However, their use was wide-spread enough that we changed course and enabled the implementation for POD types in C++. That got us most of the compatibility without a significant amount of effort, whereas we didn't have the infrastructure to handle non-PODs at that time. It wasn't a statement of intent---it just wasn't important enough to implement at the time. Looks like rjmccall's work on VLAs containing ARC-qualified pointers got us most of the way there, so it makes sense to generalize it to non-POD types.

Apr 11 2016, 11:04 PM

Apr 2 2016

doug.gregor accepted D18709: Add copyright notice to modulemap files.

Yes, this is acceptable. Please go ahead and commit.

Apr 2 2016, 8:41 PM

Mar 9 2016

doug.gregor accepted D15314: Fix a bug in unavailable checking.

LGTM

Mar 9 2016, 3:55 PM

Feb 24 2016

doug.gregor accepted D17355: [Sema] Remove assert in TreeTransform<Derived>::TransformObjCObjectType.

LGTM, sorry for the delay.

Feb 24 2016, 10:13 PM

Feb 16 2016

doug.gregor added a comment to D15314: Fix a bug in unavailable checking.

The approach and patch look okay to me, but can we give "UnavailableCheck" a less ambiguous name? For example, "TreatUnavailableAsInvalid"?

Feb 16 2016, 4:33 PM

Feb 4 2016

doug.gregor accepted D16906: [Parser] Perform CachedTokens update dependent on token consume request.

LGTM!

Feb 4 2016, 8:37 PM
doug.gregor added a comment to D16905: [Parser] Perform CachedTokens update dependent on token consume request.

LGTM!

Feb 4 2016, 8:36 PM

Jan 28 2016

doug.gregor accepted D15173: [Preprocessor] Fix assertion in AnnotatePreviousCachedTokens.

This approach looks great to me.

Jan 28 2016, 7:58 PM

Dec 24 2015

doug.gregor added a comment to D15729: Load compiler plugins in ASTUnit, too.

It looks good. Are you able to include a test for this?

Dec 24 2015, 2:01 PM

Dec 14 2015

doug.gregor added a comment to D15490: [libclang] Add a flag to create the precompiled preamble on the first parse..

For reference, you can test this by setting the environment variable LIBCLANG_TIMING in your test, and checking that the string "Precompiling preamble" shows up on first parse.

Dec 14 2015, 10:55 AM

Jul 6 2015

doug.gregor requested changes to D10949: A step towards getting libclang tests working on Windows.

It would be fantastic to get these tests running on Windows (and make libclang more usable on Windows in general).

Jul 6 2015, 8:52 AM

Feb 17 2015

doug.gregor accepted D7662: Provide the same ABI regardless of NDEBUG.

This LGTM.

Feb 17 2015, 9:25 AM

Oct 1 2014

doug.gregor added a comment to D5528: Store paren locations in IfStmt, WhileStmt, SwitchStmt..

In non-macro cases, one can extract the locations of the parentheses using the lexer. Personally, I don't think the benefits of being able to extract the locations of the parentheses efficiently or in the macro cases outweigh the disadvantages of bloating the AST further.

Oct 1 2014, 11:39 AM