Page MenuHomePhabricator

rjmccall (John McCall)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 18 2013, 11:30 AM (312 w, 4 d)

Recent Activity

Yesterday

rjmccall added a comment to D56411: [CUDA][HIP][Sema] Fix template kernel with function as template parameter.

This patch still doesn't make any sense. You don't need to do any special validation when passing a function as a template argument. When Sema instantiates the template definition, it'll rebuild the expressions that refer to the template parameter, which will trigger the normal checking for whether those expressions are illegally referencing a host function from the device, etc. All you need to do is suppress that checking (whether it happens in a template definition or not) for references from non-potentially-evaluated contexts.

If you look at line 6583 of lib/Sema/SemaTemplate.cpp, you will see clang does the check if the function needs overloading resolution. However, clang missed the check if the function does not need overloading resolution. That's why I need to add the check at line 6593. All the other stuff is just to help make this check.

why clang does not do the reference check when there is no overloading resolution?

Tue, Jan 15, 8:20 PM
rjmccall added a comment to D53153: [OpenCL] Mark kernel functions with default visibility.

It's an interesting idea at least, and it does seem like there ought to be some way to express it, at least internally. A -cc1 option is fine for your purposes, right?

Tue, Jan 15, 8:14 PM
rjmccall accepted D56746: [Fixed Point Arithmetic] Add APFixedPoint to APValue.

Thanks, LGTM.

Tue, Jan 15, 5:16 PM · Restricted Project
rjmccall accepted D53738: [Fixed Point Arithmetic] Fixed Point Addition.

I think that's the right direction, yeah.

Tue, Jan 15, 5:15 PM · Restricted Project
rjmccall added a comment to D55850: [OpenCL] Allow address spaces as method qualifiers.

Okay, so is this ready to re-review independently?

Tue, Jan 15, 3:17 PM
rjmccall added a comment to D56746: [Fixed Point Arithmetic] Add APFixedPoint to APValue.

One nit-pick, then this is good to go.

Tue, Jan 15, 3:14 PM · Restricted Project
rjmccall added a comment to D53738: [Fixed Point Arithmetic] Fixed Point Addition.

I'm fine with making this change under the assumption that we've gotten the language rule right. Even if that weren't abstractly reasonable for general language work — and I do think it's reasonable when we have a good-faith question about the right semantics — this is clearly still an experimental implementation and will be for several months yet, and hopefully it won't take that long for us to get a response.

@rjmccall Have you received a response yet? If not, do you think you have an estimate on the response time, or also mind sharing the contact information if that's ok?

I just have a coworker who's part of the committee. I think you might be over-opimistic about how quickly things get answered with the C committee, though. We should not allow our work to be blocked waiting for a response.

Tue, Jan 15, 11:12 AM · Restricted Project
rjmccall added a comment to D56735: [OpenCL] Fix overloading ranking rules to work correctly for address space coversions .

Is there a reason not to just do T1.getQualifiers() == T2.getQualifiers()?

Tue, Jan 15, 11:08 AM
rjmccall added a comment to D55868: [Fixed Point Arithmetic] Fixed Point Addition Constant Expression Evaluation.

Yeah, I would recommend splitting the APFixedPoint in APValue changes into a separate patch.

Tue, Jan 15, 11:01 AM · Restricted Project

Mon, Jan 14

rjmccall added a comment to D55394: Re-order type param children of ObjC nodes.

If I'm debugging a clang bug and calling D->dump(), I think it'll be just as clear as it used to be what the ObjCTypeParamDecl lines mean.

Mon, Jan 14, 2:50 PM
rjmccall added a comment to D55394: Re-order type param children of ObjC nodes.

This is the AST dumper. This is not a user feature.

Mon, Jan 14, 2:48 PM
rjmccall added a comment to D55394: Re-order type param children of ObjC nodes.

I don't really have an opinion about this, sorry.

Mon, Jan 14, 1:36 PM
rjmccall added a comment to D53153: [OpenCL] Mark kernel functions with default visibility.

Mostly, we don't really want the abstract visibility calculation to change whenever we see a definition.

Mon, Jan 14, 1:33 PM
rjmccall added inline comments to D54590: [compiler-rt][UBSan] Sanitization for alignment assumptions..
Mon, Jan 14, 1:32 PM · Restricted Project
rjmccall added inline comments to D54590: [compiler-rt][UBSan] Sanitization for alignment assumptions..
Mon, Jan 14, 11:25 AM · Restricted Project
rjmccall added a comment to D53153: [OpenCL] Mark kernel functions with default visibility.

I think -fvisibility=hidden isn't good enough because you want to infer hidden visibility even for external symbol references, and that's just not how global visibility works. But after this discussion, I'm prepared to accept that (1) we should have some sort of single-image compiler mode that implies that all symbols are defined within the image and (2) you can make your toolchain imply that together with -fvisibility=hidden and then have specific symbols opt in to non-hidden visibility if they need to be accessible to whatever loader / runtime you have.

It seems that explicit visibility attributes on external symbol references (e.g. __attribute__((visibility("hidden"))) extern int foo;) are respected in Clang, so I don't understand the rationale for global visibility controls not applying to them as well. Can you describe why this is the case?

Mon, Jan 14, 1:18 AM
rjmccall added a comment to D56318: [HIP] Fix size_t for MSVC environment.

It's pretty unfortunate that all these fields have to be individually called out like this. Can you move all these basic layout fields into a separate struct (which can be a secondary base class of TargetInfo) which can then just be normally copied? Anything that needs special copy semantics, like the LLVM DataLayout (do you need to copy this?) doesn't need to go into that struct, just the basic POD things that determine fundamental type layouts and semantics.

Mon, Jan 14, 12:33 AM
rjmccall accepted D56066: [OpenCL] Address space for default class members.

Great, LGTM.

Mon, Jan 14, 12:22 AM

Thu, Jan 10

rjmccall added inline comments to D56066: [OpenCL] Address space for default class members.
Thu, Jan 10, 11:43 AM
rjmccall added a comment to D56318: [HIP] Fix size_t for MSVC environment.

No, I understand that things like the function-call ABI should be different from the associated host ABI, but things like the size of long and the bit-field layout algorithm presumably shouldn't be, and that's the sort of thing that's configured by TargetInfo.

How about create a ForwardingTargegInfo which will has a pointer to AuxTarget and forward to that target if it is not null. Then let AMDGPUTargetInfo inherit from that.

Thu, Jan 10, 11:37 AM
rjmccall added a comment to D56318: [HIP] Fix size_t for MSVC environment.

No, I understand that things like the function-call ABI should be different from the associated host ABI, but things like the size of long and the bit-field layout algorithm presumably shouldn't be, and that's the sort of thing that's configured by TargetInfo.

Thu, Jan 10, 10:36 AM
rjmccall accepted D56225: [HIP] Use nul instead of /dev/null when running on windows.
Thu, Jan 10, 10:34 AM
rjmccall added a comment to D56318: [HIP] Fix size_t for MSVC environment.

If I was only concerned about size_t, your current solution would be fine. My concern is that you really need to match *all* of the associated CPU target's ABI choices, so your target really ought to be forwarding everything to that target by default and only selectively overriding it in order to support GPU-specific features. Probably the easiest way to do that is via inheritance.

Thu, Jan 10, 9:29 AM

Wed, Jan 9

rjmccall added a comment to D56411: [CUDA][HIP][Sema] Fix template kernel with function as template parameter.

This patch still doesn't make any sense. You don't need to do any special validation when passing a function as a template argument. When Sema instantiates the template definition, it'll rebuild the expressions that refer to the template parameter, which will trigger the normal checking for whether those expressions are illegally referencing a host function from the device, etc. All you need to do is suppress that checking (whether it happens in a template definition or not) for references from non-potentially-evaluated contexts.

Wed, Jan 9, 9:48 PM
rjmccall accepted D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

Sounds good.

Wed, Jan 9, 9:31 PM · Restricted Project
rjmccall added inline comments to D55844: [Fixed Point Arithmetic] Fixed Point Subtraction.
Wed, Jan 9, 4:06 PM · Restricted Project
rjmccall added a comment to D54188: [Sema] Mark target of __attribute__((alias("target"))) used for C.

Don't worry, that's a familiar mistake. :)

Wed, Jan 9, 4:04 PM
rjmccall added inline comments to D56066: [OpenCL] Address space for default class members.
Wed, Jan 9, 1:49 PM
rjmccall added inline comments to D54188: [Sema] Mark target of __attribute__((alias("target"))) used for C.
Wed, Jan 9, 1:44 PM
rjmccall added a comment to D56411: [CUDA][HIP][Sema] Fix template kernel with function as template parameter.

But why? Why do you want to limit this to just template arguments instead of all sorts of similar contexts?

Wed, Jan 9, 1:44 PM
rjmccall added a comment to D54188: [Sema] Mark target of __attribute__((alias("target"))) used for C.

I really dislike this particular idiom.

Wed, Jan 9, 12:01 PM
rjmccall added a comment to D55781: Make CC mangling conditional on the ABI version.

Okay. In my opinion, then, we should just do this unconditionally.

Wed, Jan 9, 11:16 AM

Mon, Jan 7

rjmccall added a comment to D56411: [CUDA][HIP][Sema] Fix template kernel with function as template parameter.

Sema won't necessarily have resolved a template decl when parsing a template argument list, so trying to propagate that decl down to indicate that we're resolving a template argument is not a good approach.

Mon, Jan 7, 11:45 PM
rjmccall added a comment to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

Oh, and please update the commit message to primarily talk about the changes to placeholder checking. You can discuss the impact on the repeated-use-of-weak warning in a follow-up paragraph.

Mon, Jan 7, 11:28 PM · Restricted Project
rjmccall added inline comments to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..
Mon, Jan 7, 11:15 PM · Restricted Project
rjmccall accepted D56367: [AST] Pack CXXDependentScopeMemberExpr.

Yeah, LGTM.

Mon, Jan 7, 11:10 PM · Restricted Project
rjmccall accepted D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers.

Thanks! LGTM.

Mon, Jan 7, 12:28 PM
rjmccall added a comment to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

If it's at all reasonable to just avoid doing the work multiple times, let's do that. It should also avoid duplicate diagnostics if e.g. an overload is resolved to an unavailable function.

Mon, Jan 7, 12:09 PM · Restricted Project
rjmccall added inline comments to D56066: [OpenCL] Address space for default class members.
Mon, Jan 7, 12:07 PM
rjmccall added a comment to D56367: [AST] Pack CXXDependentScopeMemberExpr.

Okay. That comment seems reasonable. Glad to hear you're on top of it. :)

Mon, Jan 7, 11:36 AM · Restricted Project
rjmccall accepted D56368: [AST] Store the results in OverloadExpr in a trailing array.

LGTM.

Mon, Jan 7, 12:09 AM · Restricted Project
rjmccall added a comment to D56367: [AST] Pack CXXDependentScopeMemberExpr.

Does the regression disappear if you make the scope specifier the first trailing object? Later trailing objects are more expensive to access, and I would imagine that the scope specifier and template arguments are more important than the other fields.

Mon, Jan 7, 12:07 AM · Restricted Project
rjmccall added inline comments to D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr .
Mon, Jan 7, 12:03 AM · Restricted Project

Sun, Jan 6

rjmccall added a comment to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

Can we find a way to preserve the rewritten expression out of DeduceAutoType so that the initialization code just happens to never see a placeholder along this path?

Sun, Jan 6, 11:58 PM · Restricted Project

Fri, Jan 4

rjmccall added a comment to D56318: [HIP] Fix size_t for MSVC environment.

Okay. Is there a reasonable way to make your targets delegate to a different TargetInfo implementation for most things so that you can generally match the host target for things like type sizes and alignments?

Fri, Jan 4, 2:38 PM
rjmccall added a comment to D56318: [HIP] Fix size_t for MSVC environment.

No, no, I understand that you're not changing pointer sizes, but this is one example of trying to match the ABI of the target environment, and I'm trying to understand how far that goes. What does it mean to be in the "MSVC" environment when you're actually just compiling for the GPU? Why are you using OS headers in the first place? Do you need struct layout to match MSVC (presumably on x86-64)? What should happen with the C++ ABI?

Fri, Jan 4, 11:12 AM
rjmccall added inline comments to D56225: [HIP] Use nul instead of /dev/null when running on windows.
Fri, Jan 4, 8:48 AM
rjmccall added a comment to D56318: [HIP] Fix size_t for MSVC environment.

What's the general idea here, that you're going to pretend to be the environment's "standard" CPU target of the right pointer width and try to match the ABI exactly? This seems like a pretty treacherous road to go down.

Fri, Jan 4, 8:43 AM
rjmccall added inline comments to D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers.
Fri, Jan 4, 8:39 AM

Thu, Jan 3

rjmccall added a comment to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

I think you could just disable the diagnostic in an unevaluated context.

The call to isUnevaluatedContext in ObjCPropertyOpBuilder::complete returns false when the type of auto __weak wp in testAuto is being deduced because the ExpressionEvaluationContextRecord on the stack isn't an unevaluated context. I can silence the warning if I can push an unevaluated context at the entry of Sema::DeduceAutoType, but it's not clear to me that it's safe to do so.

Thu, Jan 3, 10:33 PM · Restricted Project
rjmccall added a comment to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

I think you could just disable the diagnostic in an unevaluated context.

Thu, Jan 3, 8:39 PM · Restricted Project
rjmccall added a comment to D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC.

Sorry, I keep coming up with small tweaks to the documentation. These should be the last of them, so if Aaron's also satisfied, feel free to commit with those modifications.

Thu, Jan 3, 8:38 PM
rjmccall added inline comments to D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC.
Thu, Jan 3, 1:44 PM
rjmccall added a comment to D56188: Adopt SwiftABIInfo for WebAssembly..

I don't mind you committing it like this.

Thu, Jan 3, 1:32 PM
rjmccall added inline comments to D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers.
Thu, Jan 3, 8:38 AM

Wed, Jan 2

rjmccall added inline comments to D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC.
Wed, Jan 2, 10:20 PM
rjmccall added inline comments to D56066: [OpenCL] Address space for default class members.
Wed, Jan 2, 10:05 PM
rjmccall accepted D56198: [Basic] Extend DiagnosticEngine to store and format Qualifiers.

Thanks, LGTM.

Wed, Jan 2, 11:20 AM
rjmccall added inline comments to D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers.
Wed, Jan 2, 11:18 AM
rjmccall added inline comments to D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers.
Wed, Jan 2, 10:27 AM
rjmccall added inline comments to D56198: [Basic] Extend DiagnosticEngine to store and format Qualifiers.
Wed, Jan 2, 8:51 AM
rjmccall added a comment to D56188: Adopt SwiftABIInfo for WebAssembly..

...although it might be reasonable to extract the method implementations on DefaultABIInfo as helper functions so that the code can be reused without requiring a particular inheritance hierarchy.

Wed, Jan 2, 8:34 AM
rjmccall accepted D56188: Adopt SwiftABIInfo for WebAssembly..

LGTM.

Wed, Jan 2, 8:31 AM

Mon, Dec 31

rjmccall accepted D56134: [AST] Store some data of CXXNewExpr as trailing objects.
Mon, Dec 31, 1:35 PM · Restricted Project
rjmccall added a comment to D56134: [AST] Store some data of CXXNewExpr as trailing objects.

LGTM.

Mon, Dec 31, 1:35 PM · Restricted Project
rjmccall added inline comments to D55850: [OpenCL] Allow address spaces as method qualifiers.
Mon, Dec 31, 1:13 PM
rjmccall added inline comments to D56066: [OpenCL] Address space for default class members.
Mon, Dec 31, 12:49 PM
rjmccall added inline comments to D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers.
Mon, Dec 31, 12:41 PM

Fri, Dec 21

rjmccall accepted D56022: [AST] Store the arguments of CXXConstructExpr in a trailing array.

LGTM.

Fri, Dec 21, 1:43 PM · Restricted Project
rjmccall accepted D55869: Convert some ObjC retain/release msgSends to runtime calls.

It sounds like it's fine.

Fri, Dec 21, 12:35 PM
rjmccall added inline comments to D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC.
Fri, Dec 21, 12:53 AM
rjmccall added a comment to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

Okay. You may need to push an unevaluated context when doing that.

Since I'm just moving the call to CheckPlaceholderExpr to the call site, I don't think I have to push an unevaluated context there?

Hmm. Right, for the auto inference specifically it's fine because the expression is in fact evaluated: we're not just eliminating placeholders in order to resolve decltype, we're eliminating placeholders to actually figure out what's going on with the initialization.

clang currently diagnose the repeated use of weak in the following case (with or without this patch):

auto __weak wp = b.weakProp;

I find this counterintuitive, but I guess this is the expected behavior?

Fri, Dec 21, 12:22 AM · Restricted Project

Thu, Dec 20

rjmccall added a comment to D55869: Convert some ObjC retain/release msgSends to runtime calls.

This should be fine for the GNUstep runtime (the GCC runtime doesn't support ARC at all). My main concern is that it will break already-released versions of the runtime built with a newer version of clang. I can easily enable a new flag in the next release, but doing so for older ones is more problematic.

Thu, Dec 20, 2:55 PM
rjmccall added inline comments to D55850: [OpenCL] Allow address spaces as method qualifiers.
Thu, Dec 20, 2:47 PM
rjmccall added inline comments to D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers.
Thu, Dec 20, 1:40 PM
rjmccall added a comment to D55869: Convert some ObjC retain/release msgSends to runtime calls.
In D55869#1337723, @js wrote:
In D55869#1337706, @js wrote:

The ObjFW runtime itself does not contain anything about release, retain or autorelease - these are all part of ObjFW (as in the framework). As ObjFW also supports the Apple runtime, as well as mixing with Foundation code, it so far only provides objc_retain and objc_release when they are missing, to make ARC work. They just call into the corresponding methods on the object and do nothing else.

How will this change work from the Apple runtime? Will objc_retain/objc_release call the retain/release on the object if it implements its own? Should I drop my own retain/release implementation from my root object if I am compiling against the Apple runtime?

Yes, the idea is that the specialized runtime functions are fast paths for the common case, but they still fall back if the object has overridden them.

I am guessing not just overridden, but also a root class providing one? IOW, the fast path is used when the class does not have the retain or release method at all? In that case, LGTM as is.

The Apple runtime is using a bit on each realized class to track the overriding. The root class defaults to not having a custom RR, ie, it appears that its version of RR is considered equivalent to objc_retain.

Presumably that would apply to other root classes too, although i'm not 100% sure. I did see some special handling of NSObject in there too so perhaps only its RR is equivalent to objc_retain/objc_release.

Thu, Dec 20, 9:39 AM
rjmccall added a comment to D55869: Convert some ObjC retain/release msgSends to runtime calls.
In D55869#1337706, @js wrote:

Thanks for tagging me!

The ObjFW runtime itself does not contain anything about release, retain or autorelease - these are all part of ObjFW (as in the framework). As ObjFW also supports the Apple runtime, as well as mixing with Foundation code, it so far only provides objc_retain and objc_release when they are missing, to make ARC work. They just call into the corresponding methods on the object and do nothing else.

How will this change work from the Apple runtime? Will objc_retain/objc_release call the retain/release on the object if it implements its own? Should I drop my own retain/release implementation from my root object if I am compiling against the Apple runtime?

Thu, Dec 20, 9:33 AM
rjmccall added reviewers for D55869: Convert some ObjC retain/release msgSends to runtime calls: theraven, js.

Okay. That's also presumably true for open-source runtimes that support ARC; tagging David Chisnall and Jonathan Schleifer to get their input.

Thu, Dec 20, 8:55 AM

Wed, Dec 19

rjmccall added inline comments to D55844: [Fixed Point Arithmetic] Fixed Point Subtraction.
Wed, Dec 19, 8:50 PM · Restricted Project
rjmccall added a comment to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

Okay. You may need to push an unevaluated context when doing that.

Since I'm just moving the call to CheckPlaceholderExpr to the call site, I don't think I have to push an unevaluated context there?

Wed, Dec 19, 7:40 PM · Restricted Project
rjmccall added inline comments to D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC.
Wed, Dec 19, 12:45 PM
rjmccall accepted D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor..
Wed, Dec 19, 11:45 AM
rjmccall added a comment to D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor..

Thanks, LGTM. Somewhat surprised that we don't have an implicit copy of the Strong argument, but it's not a bad thing that we don't.

Wed, Dec 19, 11:45 AM
rjmccall added inline comments to D55850: [OpenCL] Allow address spaces as method qualifiers.
Wed, Dec 19, 10:44 AM

Tue, Dec 18

rjmccall added inline comments to D55868: [Fixed Point Arithmetic] Fixed Point Addition Constant Expression Evaluation.
Tue, Dec 18, 10:33 PM · Restricted Project
rjmccall added a comment to D55850: [OpenCL] Allow address spaces as method qualifiers.

You're gating on OpenCL C++ in several places in this patch, but I don't think you need to. This extension should be available to anyone using address spaces and C++.

Tue, Dec 18, 10:16 PM
rjmccall added a comment to D55869: Convert some ObjC retain/release msgSends to runtime calls.

So, once upon a time this was a problem because we were rewriting the method calls in the runtime itself. Can you explain how that's not a problem now? Do we just expect the runtime to be compiled with the right switch?

Tue, Dec 18, 10:03 PM
rjmccall added a comment to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

Sorry, please ignore my previous comment. I was looking at the wrong place.

The following code reaches Sema::BuildDecltypeType without going through ActOnDecltypeExpression:

template <typename T>
void overloaded_fn(T);
decltype(auto) v5 = &overloaded_fn<int>;

Sema::BuildDecltypeType is called from Sema::DeduceAutoType, so calling CheckPlaceholderExpr there should fix the assert when the test case above is compiled.

Tue, Dec 18, 9:59 PM · Restricted Project
rjmccall added a comment to D55662: [Sema] If CheckPlaceholderExpr rewrites a placeholder expression when the type of an auto variable is being deduced, use the rewritten expression when performing initialization of the variable..

Here are a couple of examples I found running the regression tests:

int f0(int);
float f0(float);
decltype(f0) f0_a; // this is not valid.
template <class... Ts>
int var_expr(Ts... ts);

template <class... Ts>
auto a_function(Ts... ts) -> decltype(var_expr(ts...));

template <class T>
using partial = decltype(a_function<int, T>);

int use_partial() { partial<char> n; }
template<class T> void oneT() { }
int main()
{
  decltype(oneT<int>)* fun = 0;
}

If the call to CheckPlaceholderExpr in Sema::BuildDecltypeType is moved to TreeTransform<Derived>::TransformDecltypeType and Parser::ParseDecltypeSpecifier, we can assert at the beginning of Sema::BuildDecltypeType.

Tue, Dec 18, 1:48 PM · Restricted Project
rjmccall accepted D55794: [asan] In llvm.asan.globals, allow entries to be non-GlobalVariable and skip over them.
Tue, Dec 18, 1:05 PM · Restricted Project
rjmccall added inline comments to D55794: [asan] In llvm.asan.globals, allow entries to be non-GlobalVariable and skip over them.
Tue, Dec 18, 12:26 PM · Restricted Project
rjmccall added inline comments to D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor..
Tue, Dec 18, 12:04 PM
rjmccall accepted D55771: [AST] Store the callee and argument expressions of CallExpr in a trailing array..

Thanks, looks great.

Tue, Dec 18, 8:18 AM · Restricted Project

Mon, Dec 17

rjmccall accepted D55802: Change CGObjC to use objc intrinsics instead of runtime methods.

You're making intrinsics with weak_external linkage? I feel like that's going to be unnecessarily awkward in the future, but okay.

Mon, Dec 17, 9:43 PM
rjmccall added inline comments to D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor..
Mon, Dec 17, 9:22 PM
rjmccall added a comment to D55771: [AST] Store the callee and argument expressions of CallExpr in a trailing array..

I agree with not packing the argument count in.

Mon, Dec 17, 8:06 PM · Restricted Project
rjmccall added a reviewer for D55781: Make CC mangling conditional on the ABI version: probinson.

I think the core question is whether there are any vendors who care about ABI stability for these attributes who don't care about the bugfix. Speaking for Apple, we would rather have the bugfix. Sony might have different thoughts; CC'ing Paul.

Mon, Dec 17, 7:55 PM
rjmccall added a comment to D54862: [OpenCL] Add generic AS to 'this' pointer.

I'm also a bit confused about the semantics that this patch is applying to function types. It mostly seems to concern the extra trailing Qualifiers on CXXMethodDecl to store the addrspace quals, but in some places (SemaType:4842, SemaDecl:3198) it seems to be applying the address space to the function type itself, which certainly seems like something else to me. A function with an address space qualified type would (to me, at least) be a function *located* in that address space, not one qualified to take a this from that address space.

Mon, Dec 17, 3:55 PM
rjmccall added a comment to D55794: [asan] In llvm.asan.globals, allow entries to be non-GlobalVariable and skip over them.

Thanks! I think the only real question here is whether the entry should be *skipped* if it isn't directly a global variable or if you should stripPointerCasts() to try to find the global.

Mon, Dec 17, 3:49 PM · Restricted Project
rjmccall added inline comments to D54862: [OpenCL] Add generic AS to 'this' pointer.
Mon, Dec 17, 12:10 PM