On Nov 11, 2010, at 12:44 PM, Argyrios Kyrtzidis wrote:
> On Nov 10, 2010, at 10:06 PM, Douglas Gregor wrote:
>
>>
>> On Nov 10, 2010, at 9:34 PM, Argyrios Kyrtzidis wrote:
>>
>>> Diagnostic pragmas are broken because we don't keep track of the diagnostic
>>> state changes and we only check the current/latest state.
>>> Problems manifest if a diagnostic is emitted for a source line that has
>>> different diagnostic state than the current state; this can affect
>>> a lot of places, like C++ inline methods, template instantiations, the
>>> lexer, etc. For example:
>>>
>>> struct S {
>>> #pragma clang diagnostic push
>>> #pragma clang diagnostic ignored "-Wtautological-compare"
>>> void m() { int b = b==b; }
>>> #pragma clang diagnostic pop
>>> };
>>>
>>> the pragmas do not work for suppressing the warning in the above case.
>>>
>>> The attached patch fixes the issue by having the Diagnostic object keep
>>> track of the source location of the pragmas so that it is able to know what
>>> is the diagnostic state at any given source location.
>>
>> This is really cool, Argiris!
>>
>> @@ -365,33 +376,56 @@ void Diagnostic::ReportDelayed() {
>> DelayedDiagArg2.clear();
>> }
>>
>> +Diagnostic::DiagStatePointsTy::iterator
>> +Diagnostic::GetDiagStatePointForLoc(FullSourceLoc Loc) const {
>> + assert(!DiagStatePoints.empty());
>> + assert(DiagStatePoints.front().Loc.isInvalid() &&
>> + "Should have created a DiagStatePoint for command-line");
>> +
>> + if (Loc.isInvalid())
>> + return DiagStatePoints.begin();
>>
>> I guess I was expecting that we would return the last DiagStatePoint for an
>> invalid location (so that we get the most recent state), rather than the
>> first.
>
> Yes, that is more appropriate, I'll make some changes.
>
>>
>> diff --git a/lib/Lex/PPLexerChange.cpp b/lib/Lex/PPLexerChange.cpp
>> index 4a40405..51b6dae 100644
>> --- a/lib/Lex/PPLexerChange.cpp
>> +++ b/lib/Lex/PPLexerChange.cpp
>> @@ -252,12 +252,14 @@ bool Preprocessor::HandleEndOfFile(Token &Result, bool
>> isEndOfMacro) {
>>
>> // This is the end of the top-level file. If the diag::pp_macro_not_used
>> // diagnostic is enabled, look for macros that have not been used.
>> - if (getDiagnostics().getDiagnosticLevel(diag::pp_macro_not_used) !=
>> - Diagnostic::Ignored) {
>> - for (macro_iterator I = macro_begin(false), E = macro_end(false);
>> - I != E; ++I)
>> - if (!I->second->isUsed())
>> - Diag(I->second->getDefinitionLoc(), diag::pp_macro_not_used);
>> + for (macro_iterator I = macro_begin(false), E = macro_end(false);
>> + I != E; ++I) {
>> + if (I->second->isUsed())
>> + continue;
>> + FullSourceLoc DefLoc(I->second->getDefinitionLoc(), getSourceManager());
>> + if (getDiagnostics().getDiagnosticLevel(diag::pp_macro_not_used,
>> DefLoc) !=
>> + Diagnostic::Ignored)
>> + Diag(DefLoc, diag::pp_macro_not_used);
>> }
>>
>> It's unfortunate this fixing this check means that we always have to iterate
>> over all of the macros. In particular, I wonder if this causes
>> deserialization of macros of a PCH file, which would be a performance
>> killer. Could you check?
>
> That is badness.
>
> How about this; Since we know at macro creation time whether it needs to be
> checked or not, keep that information (CheckUsed) in MacroInfo.
> For de/serialization keep in PCH the set of macros that need checking
> (CheckUsed && !IsUsed) and only deserialize these macros.
Sounds good, thanks!
> This also improves a bit the current situation where we deserialize all
> macros if the diagnostic is enabled.
Yes!
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
