- User Since
- Jul 17 2012, 3:26 PM (365 w, 3 m)
Jan 23 2019
One minor request about regarding a dyn_cast, but this looks like the right approach. Thank you!
Apr 18 2018
Apr 17 2018
Feb 13 2018
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.
Oct 24 2017
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.
Sep 14 2017
Looks good to me; sorry for the delay.
Aug 10 2017
LGTM, thank you!
Jul 26 2017
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 5 2017
Committed as r307196 and r307197
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 4 2017
Jun 28 2017
Mar 29 2017
Jan 5 2017
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.
Dec 8 2016
Nov 10 2016
Nov 9 2016
I don't see a better way to do this. Nice meta programming hack.
Looks good! I appreciate the refactoring of recordNullabilitySeen
This is looking great, Jordan.
Nov 1 2016
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.
Oct 19 2016
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.
Sep 12 2016
Yep, this refactor looks good!
Ahhh, much cleaner. Thanks!
This looks great, thank you!
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.
Aug 25 2016
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 23 2016
Aug 19 2016
I think most of the complexity here will fold away when ObjCTypeParamType becomes sugar for the underlying ObjCObjectPointerType.
A couple of comments above, but this is looking very good.
This refactor LGTM.
Jul 19 2016
Jul 14 2016
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.
I think this check should go into SemaChecking.cpp
May 31 2016
Yes, that's a LGTM, sorry for being unclear.
May 30 2016
Yeah, this looks like the right approach. PCH follows the same rules as modules when it comes to newer information shadowing imported information.
May 27 2016
LGTM, sorry for the delay!
Apr 29 2016
Mostly looks good, but see my comment about MapVector invalidation.
Apr 25 2016
Yeah, this LGTM.
Apr 11 2016
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 2 2016
Yes, this is acceptable. Please go ahead and commit.
Mar 9 2016
Feb 24 2016
LGTM, sorry for the delay.
Feb 16 2016
The approach and patch look okay to me, but can we give "UnavailableCheck" a less ambiguous name? For example, "TreatUnavailableAsInvalid"?
Feb 4 2016
Jan 28 2016
This approach looks great to me.
Dec 24 2015
It looks good. Are you able to include a test for this?
Dec 14 2015
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.
Jul 6 2015
It would be fantastic to get these tests running on Windows (and make libclang more usable on Windows in general).
Feb 17 2015
Oct 1 2014
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.