On Aug 17, 2012, at 8:15 , Andy Gibbs <[email protected]> wrote:
> On Thursday, August 16, 2012 8:12 PM, Jordan Rose wrote:
>> On Aug 16, 2012, at 11:00 , Andy Gibbs wrote:
>>> ---When we get a diagnostic---
>>> if (is a macro expansion)
>>> ignore [[big culprit this one!]]
>>
>> Why not look at where the macro is instantiated?
>
> Is it necessary? A macro expansion itself will not contain comments,
> and hence no directive?... (that's right isn't it?)
I'll agree that directives should not come from macros, but a macro
/instantiation/ can result in diagnostics (that's this case), in which case we
definitely want to make sure that the file containing the macro instantiation
location (not its definition location!) has been parsed for directives. I'm not
sure we'll ever actually see an issue like this, but it seems better than
dropping it on the floor.
I just realized the DiagnosticsEngine has a SourceManager. Does it make sense
for you to use that instead of keeping track of it separately?
Nits:
+ class UnparsedFileStatus
+ : private llvm::PointerIntPair<const FileEntry*, 1, bool> {
+ typedef llvm::PointerIntPair<const FileEntry*, 1, bool> base;
Probably should have commented on this earlier, but private inheritance is
pretty rare in LLVM. We tend to just use a member variable instead.
+ ParsedFiles.insert(std::make_pair(FID, SM.getFileEntryForID(FID)));
Since there's no search or result checking here, I think it's a lot cleaner to
use subscripting, i.e. "ParsedFiles[FID] …"
+ // Add FileID to unparsed.
+ UnparsedFiles.insert(std::make_pair(FID,
+ UnparsedFileStatus(SM.getFileEntryForID(FID),
+ FoundDirectives)));
Ditto.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
