In http://reviews.llvm.org/D9714#171408, @rsmith wrote:

> First off: I'm not happy about having this extension in upstream clang until 
> we have a strong indication that this is the direction that will end up 
> standardized. For now, I'd recommend maintaining this as a clang fork on 
> github or similar.


Will do.

> With that said, I'm going to review this as if for upstream clang.


Thanks! (The point of doing this is to get feedback on implementation-related 
issues)

> I don't like that you create two variables here. We try to maintain as much 
> source fidelity as we can, and I think we can do better here -- how about 
> instead introducing a new form of expression that represents "stack-allocate 
> a certain amount of memory" (with a subexpression for the initialization, if 
> you allow these variables to have an initializer), much like 
> MaterializeTemporaryExpr does for a SD_Automatic temporary, but parameterized 
> by an expression specifying the array size? Then create just a single 
> expression of the std::arb type, initialized by that expression.

> 

> You should also introduce a Type subclass to represent type sugar for the ARB 
> type, so that we can model that int[n] desugars to std::arb<int> but should 
> be pretty-printed as an array type.


That makes sense (and this is very similar to how we currently handle 
std::initializer_list).

Any opinion on:

1. Should we bother with the access-control-overriding init type? We could just 
make the constructor public but make it UB if the user calls it (it would be 
implementation-specific anyhow).
2. Should I make std::arb manage the lifetime of the objects directly? If it 
just takes a special allocation expression maybe that's more natural? I'd like 
to not force extra work for POD types (but I imagine I could use some 
enable_if/is_pod logic to leave PODs uninitialized).
3. The automatically-included header (or similar) with a simpler class, or just 
requiring the header and making the class more fully-featured?

Thanks again!


http://reviews.llvm.org/D9714

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to