[cfe-dev] Two patches

[Ted Kremenek]

I've applied the ParentMap patch here:

  http://llvm.org/viewvc/llvm-project?view=rev&revision=119181

For the second patch, I really don't think we should be adding these fields to LangOptions.  I understand the convenience for putting them there, but LangOptions shouldn't be some general object that we stuff random flags into because it is the most convenient context object. It's fundamentally a layering violation as well.  LangOptions is in libBasic, while these flags only apply to an implementation detail of libAST.   If we need a new context object, maybe we should do that, or possibly add these to ASTContext.

On Nov 15, 2010, at 8:30 AM, Olaf Krzikalla wrote:

> Hi @clang,
> 
> I've attached two patches and hope that both will be soon or later committed. I've subdivided them, because the first one is very tiny and was already discussed here.
> In particular:
> 
> 1.
> parentmap.patch adds a function "addStmt" to ParentMap which actually adds a statement or updates the parent-children relations of an already existing statement. The comment to "addStmt" respects the remarks that some of you made at an earlier attempt ("A patch for printing policy and other stuff").
> 
> 2.
> rewrite_options.patch adds two code rewrite options to the pretty printer of the clang frontend:
> -rewrite-indent <value> Indentation of formatted rewrite output -rewrite-style <value>  Style of formatted rewrite output: KR|ANSI
> 
> The ANSI style now works with the majority of statements and declarations with the exception of some ObjC constructs. I'm not sure whether there is a de facto standard for ObjC syntax or whether there are any syntactic restrictions in ObjC. Maybe someone familiar with ObjC can extend the pretty printer in order to respect the rewrite style for ObjC too.
> However, there are some remarks necessary:
> - A namespace declaration is always rewritten in KR mode. This is by intent.
> - There are two other changes hidden in the patch: PrinterHelper::handledStmt got the current indentation level as an argument and I made Rewriter::getMappedOffset public. Both changes should be harmless.
> - The IMHO most arguable point is the placement of the two options. They are now members of LangOptions, which is the pragmatic solution. Leaving them in PrintingPolicy would have caused a lot of refactoring during argument parsing (at that time there is no PrintingPolicy yet). Placing them in FrontendOptions (the more natural place) would have not only introduced more dependencies but also over-complicated the construction of PrintingPolicy.
> 
> 
> I hope these patches make it into clang (my own source-to-source
> transformer relies on them).
> 
> 
> Best Olaf
> <rewrite_options.patch><parentmap.patch>_______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev