On Saturday, August 11, 2012 6:03 PM, Vassil Vassilev wrote: > On 8/10/12 8:22 PM, Andy Gibbs wrote: >> [...snip...] >> I need to look into this for you. Certainly this is a use-case I didn't >> consider -- my apologies. Would you be willing to post some sample code >> and I'll work a proper test-case from it so that I can include this use- >> case directly into clang's test-suite. > Unfortunately it is not easy to construct a test case in clang. The > closest similar use in clang is when you start it without any file to > compile but with "-". However it is by far different from what we have. > IMO the best is just download cling and run make test (the same way you do > it for clang). Sorry I can't be more helpful here, but I can't come up > with anything else.
That's ok. I've get a copy of cling and do as you say. > + if (const FileEntry *E = SM.getFileEntryForID(SM.getFileID(Loc))) > + FilesList.insert(E); > + } > > Isn't that part bound to the SourceManager instance? No, FileEntry instances are global (cached). > I don't understand why you even need to split the logic into release and > debug part (#ifdef NDebug). What is the intent of doing it? The debug part is an expensive secondary check that all expected-* directives have been captured. This was very important in picking up the various places inside clang where holes previously existed. In the release build such a check is not necessary since in the debug build the check has shown that everything is captured. As such, unless there is a really good and compelling reason, I would be loathed to remove this secondary check. After all, one could (and I would!) argue that the problem you are having here is exactly why such a check is necessary. The answer is to fix your problem, not remove the check that makes it (and maybe other things) visible! >> In the meantime, attached is a patch (** not to be committed, Jordan! **) >> that simply changes FilesParsedForDirectives from FileEntry* to FileID. >> Like I said, PCH test-cases now fail, so its not a final solution, but >> I'd be interested to know if you can use "appendParsedFile" to get round >> your problem. > Yes this patch works perfectly for our use case. As a matter of fact I did > the same, but I saw couple of failures in clang (as you said: modules and > PCHs) and I didn't have time to think how to fix it. That is why I want to > discuss it first. Can't we add an extra check whether the FileID has > FileEntry and that FileEntry is PCH or something like that? I have a solution in mind which was forming when I posted this patch, but I also want to check it thoroughly and generate a test-case which can be part of the clang test-suite ongoing which encapsulates your usage. Can you give me a few days? Thanks Andy _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
