ilovepi wrote:

U

> I like Mustache! Just took a quick look at this following your Discourse post.
> 
> Do the custom allocators make any significant performance difference 
> (measure)? Wouldn't want to prematurely optimize. They're the kind of thing 
> that make maintenance annoying and can be hard to remove later.
> 

Using the arena allocator simplifies the code greatly. We don't need RAII, and 
the lifetimes of these allocations are well scoped to the lifetime of the 
arena. It's also the pattern we use elsewhere in the project for 
parsing/tokenizing, which is why I suggested the pattern in the first place.

> Same with `SmallString<0>` / `SmallVector`, in some places `std::string` may 
> be sufficient. I think this first implementation can afford to be as simple 
> as possible, specific implementation optimizations can be added later.
> 

Those are already part of the advice in the programmer's manual. Using them 
also allows us to benefit from additional testing via expensive checks/reverse 
iteration, etc.

> Should `Token` and `ASTNode` be present in the `Mustache.h` header at all?

That's a fair point. For the API surface we're exposing, it may be better to 
make those private to the implementation, or at least not part of the public 
header. To some extend that's dependent on how the library will be used in 
practice, though. @PeterChou1 what are your thoughts here?

https://github.com/llvm/llvm-project/pull/105893
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to