Richard Smith <rich...@metafoo.co.uk> writes: > I don't think this actually solves the problem: > InclusionRewriter::InclusionDirective also copies around a FileChange with an > uninitialized FileType, so I think you'll need to initialize it or give it a > copy constructor that doesn't copy the FileType until the Id has been set. > > Ultimately, it seems like the design here is at fault; maybe we should be > holding onto the HashLoc and Imported when we reach InclusionDirective, and > not inserting the value into the map until we reach FileChanged and know the > complete value we want to insert?
You're right - the way the FileChange type was being used here was a bit of a mess. I reworked all of this to simplify it and avoid needing sometimes-initialized fields in r241140. > On Tue, Jun 30, 2015 at 4:58 PM, Justin Bogner <m...@justinbogner.com> wrote: > > ping > > Justin Bogner <m...@justinbogner.com> writes: > > If a FileChange has an invalid file, the FileType won't be initialized, > > so it's undefined to pass it into Process like this. We immediately bail > > out because the FileID is invalid in that case, so we can avoid the > > issue by checking that first. > > > > The alternative is to initialize FileType in FileChange's constructor, > > but it's not clear if there's a reasonable default. Should I do that > > instead, or is this good to commit? > > > > commit bea2df0fed8f0ce08daa23d0dfc21aead1d12e63 > > Author: Justin Bogner <m...@justinbogner.com> > > Date: Mon Jun 22 12:45:00 2015 -0700 > > > > Frontend: Avoid some UB by checking a FileChange's validity before a > call > > > > If a FileChange has an invalid file, the FileType won't be > > initialized, so it's undefined to pass it into Process like this. We > > immediately bail out because the FileID is invalid in that case, so > we > > can avoid the issue by checking that first. > > > > diff --git a/lib/Frontend/Rewrite/InclusionRewriter.cpp b/lib/Frontend/ > Rewrite/InclusionRewriter.cpp > > index b9ea051..2ab18cf 100644 > > --- a/lib/Frontend/Rewrite/InclusionRewriter.cpp > > +++ b/lib/Frontend/Rewrite/InclusionRewriter.cpp > > @@ -439,7 +439,8 @@ bool InclusionRewriter::Process(FileID FileId, > > WriteImplicitModuleImport(Change->Mod); > > > > // else now include and recursively process the file > > - } else if (Process(Change->Id, Change->FileType)) { > > + } else if (!Change->Id.isInvalid() && > > + Process(Change->Id, Change->FileType)) { > > // and set lineinfo back to this file, if the nested > one was > > // actually included > > // `2' indicates returning to a file (after having > included > _______________________________________________ > cfe-commits mailing list > cfe-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits