[cfe-commits] r139595 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/JumpDiagnostics.cpp test/SemaCXX/MicrosoftExtensions.cpp

[Francois Pichet]

On Tue, Sep 13, 2011 at 11:33 AM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Sep 13, 2011, at 3:26 AM, Francois Pichet wrote:
>
>> Author: fpichet
>> Date: Tue Sep 13 05:26:51 2011
>> New Revision: 139595
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=139595&view=rev
>> Log:
>> In Microsoft mode, downgrade "goto into protected scope" from error to warning if we are jumping over a variable initialization via a goto.
>>
>> This fixes a few errors when parsing MFC code with clang.
>>
>> Modified:
>>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>    cfe/trunk/lib/Sema/JumpDiagnostics.cpp
>>    cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=139595&r1=139594&r2=139595&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 13 05:26:51 2011
>> @@ -2657,6 +2657,8 @@
>>   InGroup<UnusedLabel>, DefaultIgnore;
>>
>> def err_goto_into_protected_scope : Error<"goto into protected scope">;
>> +def warn_goto_into_protected_scope : ExtWarn<"goto into protected scope">,
>> +  InGroup<Microsoft>;
>> def err_switch_into_protected_scope : Error<
>>   "switch case is in protected scope">;
>> def err_indirect_goto_without_addrlabel : Error<
>>
>> Modified: cfe/trunk/lib/Sema/JumpDiagnostics.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/JumpDiagnostics.cpp?rev=139595&r1=139594&r2=139595&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/JumpDiagnostics.cpp (original)
>> +++ cfe/trunk/lib/Sema/JumpDiagnostics.cpp Tue Sep 13 05:26:51 2011
>> @@ -76,8 +76,8 @@
>>   void VerifyIndirectJumps();
>>   void DiagnoseIndirectJump(IndirectGotoStmt *IG, unsigned IGScope,
>>                             LabelDecl *Target, unsigned TargetScope);
>> -  void CheckJump(Stmt *From, Stmt *To,
>> -                 SourceLocation DiagLoc, unsigned JumpDiag);
>> +  void CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
>> +                 unsigned JumpDiag, unsigned JumpDiagWarning);
>>
>>   unsigned GetDeepestCommonScope(unsigned A, unsigned B);
>> };
>> @@ -476,7 +476,8 @@
>>     // With a goto,
>>     if (GotoStmt *GS = dyn_cast<GotoStmt>(Jump)) {
>>       CheckJump(GS, GS->getLabel()->getStmt(), GS->getGotoLoc(),
>> -                diag::err_goto_into_protected_scope);
>> +                diag::err_goto_into_protected_scope,
>> +                diag::warn_goto_into_protected_scope);
>>       continue;
>>     }
>>
>> @@ -484,7 +485,7 @@
>>     if (IndirectGotoStmt *IGS = dyn_cast<IndirectGotoStmt>(Jump)) {
>>       LabelDecl *Target = IGS->getConstantTarget();
>>       CheckJump(IGS, Target->getStmt(), IGS->getGotoLoc(),
>> -                diag::err_goto_into_protected_scope);
>> +                diag::err_goto_into_protected_scope, 0);
>>       continue;
>>     }
>
> Does this 0 need to be diag::warn_goto_into_protected_scope?

IndirectGoto is a gcc extension not supported by MSVC.

>
>> @@ -493,7 +494,7 @@
>>          SC = SC->getNextSwitchCase()) {
>>       assert(LabelAndGotoScopes.count(SC) && "Case not visited?");
>>       CheckJump(SS, SC, SC->getLocStart(),
>> -                diag::err_switch_into_protected_scope);
>> +                diag::err_switch_into_protected_scope, 0);
>>     }
>>   }
>> }
>> @@ -660,10 +661,20 @@
>>       S.Diag(Scopes[I].Loc, Scopes[I].InDiag);
>> }
>>
>> +/// Return true if a particular error+note combination must be downgraded
>> +/// to a warning in Microsoft mode.
>> +static bool IsMicrosoftJumpWarning(unsigned JumpDiag, unsigned InDiagNote)
>> +{
>> +    return (JumpDiag == diag::err_goto_into_protected_scope &&
>> +           (InDiagNote == diag::note_protected_by_variable_init ||
>> +            InDiagNote == diag::note_protected_by_variable_nontriv_destructor));
>> +}
>> +
>> +
>> /// CheckJump - Validate that the specified jump statement is valid: that it is
>> /// jumping within or out of its current scope, not into a deeper one.
>> -void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To,
>> -                                 SourceLocation DiagLoc, unsigned JumpDiag) {
>> +void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
>> +                             unsigned JumpDiagError, unsigned JumpDiagWarning) {
>>   assert(LabelAndGotoScopes.count(From) && "Jump didn't get added to scopes?");
>>   unsigned FromScope = LabelAndGotoScopes[From];
>>
>> @@ -679,19 +690,30 @@
>>   if (CommonScope == ToScope) return;
>>
>>   // Pull out (and reverse) any scopes we might need to diagnose skipping.
>> -  SmallVector<unsigned, 10> ToScopes;
>> -  for (unsigned I = ToScope; I != CommonScope; I = Scopes[I].ParentScope)
>> -    if (Scopes[I].InDiag)
>> -      ToScopes.push_back(I);
>> -
>> -  // If the only scopes present are cleanup scopes, we're okay.
>> -  if (ToScopes.empty()) return;
>> -
>> -  S.Diag(DiagLoc, JumpDiag);
>> -
>> -  // Emit diagnostics for whatever is left in ToScopes.
>> -  for (unsigned i = 0, e = ToScopes.size(); i != e; ++i)
>> -    S.Diag(Scopes[ToScopes[i]].Loc, Scopes[ToScopes[i]].InDiag);
>> +  SmallVector<unsigned, 10> ToScopesError;
>> +  SmallVector<unsigned, 10> ToScopesWarning;
>> +  for (unsigned I = ToScope; I != CommonScope; I = Scopes[I].ParentScope) {
>> +    if (S.getLangOptions().Microsoft &&
>> +        IsMicrosoftJumpWarning(JumpDiagError, Scopes[I].InDiag))
>> +      ToScopesWarning.push_back(I);
>> +    else if (Scopes[I].InDiag)
>> +      ToScopesError.push_back(I);
>> +  }
>> +
>> +  // Handle warnings.
>> +  if (!ToScopesWarning.empty()) {
>> +    S.Diag(DiagLoc, JumpDiagWarning);
>> +    for (unsigned i = 0, e = ToScopesWarning.size(); i != e; ++i)
>> +      S.Diag(Scopes[ToScopesWarning[i]].Loc, Scopes[ToScopesWarning[i]].InDiag);
>> +  }
>> +
>> +  // Handle errors.
>> +  if (!ToScopesError.empty()) {
>> +    S.Diag(DiagLoc, JumpDiagError);
>> +    // Emit diagnostics note for whatever is left in ToScopesError.
>> +    for (unsigned i = 0, e = ToScopesError.size(); i != e; ++i)
>> +      S.Diag(Scopes[ToScopesError[i]].Loc, Scopes[ToScopesError[i]].InDiag);
>> +  }
>> }
>
> This will mean that we get the warnings/errors in a different order (in Microsoft mode) than we get the errors in non-Microsoft mode. Could you tweak this so that the warning/error order is consistent?
>

Not sure I understand what you mean here. There is only 1 diag error
possible here (associated with multiple notes) + 1 possible diag
warning in Microsoft mode.

The loop is to print the notes and they must be associated with either
the error or the warning.