[cfe-dev] Recombining AST Nodes
John Leidel via cfe-dev
cfe-dev at lists.llvm.org
Sun Feb 7 06:37:35 PST 2021
David, thanks again for helping debug this. Your fix required some
additional work to the template definitions for RebuildElaboratedType,
but this was otherwise successful! If anyone is interested in the
patch, we have a single commit on our (modified) Clang 9.0.1 repo
here: https://github.com/tactcomplabs/clang-upc/commit/d30a2cc74387bc5bc613275368d221e3fbdbddce
The patch is simple enough that it can easily be merged into vanilla
Clang 9.0.1 repos.
David, I sincerely appreciate your ongoing help in debugging this.
best
john
On Fri, Feb 5, 2021 at 6:18 PM David Rector <davrecthreads at gmail.com> wrote:
>
>
>
> 1. Go to your local lib/Sema/TreeTransform.h
> 2. Add an `OwnedTagDecl` param to `RebuildElaboratedType`; i.e. https://github.com/llvm-mirror/clang/blob/master/lib/Sema/TreeTransform.h#L979 is changed to:
>
> ```
> QualType RebuildElaboratedType(SourceLocation KeywordLoc,
> ElaboratedTypeKeyword Keyword,
> NestedNameSpecifierLoc QualifierLoc,
> QualType Named,
> TagDecl *OwnedTagDecl) {
> return SemaRef.Context.getElaboratedType(Keyword,
> QualifierLoc.getNestedNameSpecifier(),
> Named,
> OwnedTagDecl);
> }
> ```
>
> 3. Pass the proper arg in TransformElaboratedType, i.e. change https://github.com/llvm-mirror/clang/blob/master/lib/Sema/TreeTransform.h#L6197 to
>
> ```
> TagDecl *NewOwnedTagDecl = T->getOwnedTagDecl() ? TransformDecl(SourceLocation(), T->getOwnedTagDecl()) : nullptr;
> Result = getDerived().RebuildElaboratedType(TL.getElaboratedKeywordLoc(),
> T->getKeyword(),
> QualifierLoc, NamedT,
> NewOwnedTagDecl);
> ```
>
> See if that works.
>
>
> On Feb 5, 2021, at 6:48 PM, John Leidel <john.leidel at gmail.com> wrote:
>
> David, for reference, the code we're porting (referenced above) is
> being merged up to Clang 9.0.1 (pre mono-repo). The original
> transform functions worked in Clang 3.9.0, so this may be a
> regression.
>
> That being said, I'm not sure I follow your potential solution.
> Apologies for the naive response. I don't usually work in the AST.
>
> best
> john
>
> On Fri, Feb 5, 2021 at 5:26 PM David Rector <davrecthreads at gmail.com> wrote:
>
>
> John I think you’ve identified a bug: ElaboratedTypes are not being rebuilt with any OwnedTagDecl in TreeTransform.
>
> The last arg to getElaboratedType is the OwnedTagDecl which defaults to nullptr — and that default is being passed here:
>
> https://github.com/llvm-mirror/clang/blob/master/lib/Sema/TreeTransform.h#L983
>
> Perhaps you can try fixing that so RebuildElaboratedType requires an NewOwnedTagDecl arg, and pass it the transformed version of the old OwnedTagDecl, and see if that gets it to print properly.
>
> - Dave
>
>
> On Feb 5, 2021, at 5:04 PM, John Leidel <john.leidel at gmail.com> wrote:
>
> David, thanks again for the response. We are indeed transforming the
> AST using combinations of new Decl's via Create* and tree transforms.
> This is code we're helping to maintain (not the original authors), so
> its a bit disjunct.
>
> The original code is here if you're interested:
> https://github.com/Intrepid/upc2c/blob/master/Transform.cpp#L2064
>
> Basically, in the FieldDecl iterator loop, the RecordDecl's for the
> struct definition are stored in the new DeclContext, but the
> FieldDecl's are recreated. I'm curious whether we check for an
> ElaboratedType and if it exists for the target FieldDecl, utilize the
> existing Decl for the new DeclContext.
>
> Thoughts?
>
> best
> john
>
>
> On Fri, Feb 5, 2021 at 3:23 PM David Rector <davrecthreads at gmail.com> wrote:
>
>
> No that is not a general process — I assumed you had modified the AST by replacing the existing RecordDecl with a new one, and were printing the whole AST, due to the behavior you are getting, and given your description of your visitors as "transforming" the code, which RecursiveASTVisitors don’t do by default — they just visit it, not modifying anything. (Unless you're overriding TreeTransformer instead?)
>
> But you might also be getting this behavior if you are manually printing each Decl you want, in a RecursiveASTVisitor. If this is the case, remember, struct { int i } s; is two distinct Decls, and that loop in DeclPrinter::VisitDeclContext that I linked to is necessary to get the proper printing behavior for such cases.
>
> There’s not quite enough information to know exactly what you need to do, but let’s start with the advice that that if you are manually printing individual Decls (e.g. D->print(OS)), that won’t work for your case — you need to instead handle a DeclContext as a whole and perhaps just copy and paste the important parts of DeclPrinter::VisitDeclContext and other parts of DeclPrinter. If you’re still confused share a few more details — are you using RecursiveASTVisitor, are you printing from it, do you just want to print an output file etc.
>
> Good luck,
>
> Dave
>
> On Feb 5, 2021, at 2:18 PM, John Leidel <john.leidel at gmail.com> wrote:
>
> David, thanks for the response. Please excuse my ignorance, but I'm a
> bit confused as to the order this needs to occur. I'll try to
> reiterate what you said as follows:
>
> 1. If a TagDecl that is !isFreeStanding() is found, create a new
> RecordDecl to represent it and add it to the DeclContext
> 2. Save a reference to the newly created RecordDecl
> 3. The next FieldDecl whose TagDecl matches the new RecordDecl should
> be found; at this point create a new ElaboratedType using that TagDecl
> 4. Create a new FieldDecl with the new ElaboratedType as the type
>
> Is this the general process?
>
> On Fri, Feb 5, 2021 at 9:57 AM David Rector <davrecthreads at gmail.com> wrote:
>
>
> Hi John,
>
> IIUC you are creating a new RecordDecl, manually replacing the old one with the new one in its DeclContext, and then printing it. As you seem to recognize this is the problematic line (recall this is from that DeclPrinter code I referenced the last time):
>
> https://github.com/llvm-mirror/clang/blob/master/lib/AST/DeclPrinter.cpp#L410
>
> You need the FieldDecl (which should be the very next Decl* in the DeclContext after the old RecordDecl *) to have an elaborated type whose owned tag decl is the new RecordDecl you have created (which will be `Decls[0]` in that code); if it is thus, and if the RecordDecl you created is marked as !isFreeStanding(), it should print as you expect, I think.
>
> To create that type, use the ASTContext::getElaboratedType(…) method, which accepts the OwnedTagDecl as an arg. (ASTContext manages the types to ensure there are not multiple copies of the same Type* floating around, IIUC.)
>
> Then, I would probably also replace the existing FieldDecl with a newly Created one, in the same manner you replaced the RecordDecl *, only now Create it with the new type.
>
> Hope that works — good luck,
>
> Dave
>
> On Feb 4, 2021, at 9:59 AM, John Leidel via cfe-dev <cfe-dev at lists.llvm.org> wrote:
>
> I'm in the process of debugging (again) issues with anonymous struct
> decls. I have the following basic structures located in a C source
> file that I'm attempting to transform. I want to preserve the layout
> of the structs.
>
> struct{
> int A;
> }S;
>
> struct{
> int B;
> struct{
> int C;
> }T;
> }R;
>
> When I use my AST visitors, I can pick up the RecordDecl for the
> struct definitions and the VarDecl for the variable definitions, but
> the transformed code is outputted as:
> struct{
> int A;
> };
> struct (anonymous) S;
>
> I can view the RecordDecl as as TagDecl and save off a reference to it
> if it is !isFreeStanding(), but how does one go about recombining the
> declaration of "S" and the original TagDecl?
>
> For the VarDecl ("S" in this case), I can determine whether or not its
> an "ElaboratedType", but I'm not sure how to utilize the saved
> reference to the RecordDecl:
>
> if(isa<ElaboratedType>(SemaRef.Context.getBaseElementType(MyVarDecl))) {
> /// how do i recombine the VarDecl and the RecordDecl/TagDecl?
> }
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
>
>
More information about the cfe-dev
mailing list