[clang-tidy] fix for NOLINT after macro expansion
Summary:
When having
``` c++
    #define MACRO code-with-warning
    MACRO; // NOLINT
```
clang-tidy would still show the warning, because
it searched for "NOLINT" only in the first line,
not on the second.
This caused e.g. https://llvm.org/bugs/show_bug.cgi?id=29089
(where the macro was defined in a system header). See also
the added test cases.
Now clang-tidy looks at the line of macro invocation and every line
of macro definition for a NOLINT comment.
Reviewers: alexfh, aaron.ballman, hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D24845
llvm-svn: 282330
			
			
This commit is contained in:
		
							parent
							
								
									907cde3cc2
								
							
						
					
					
						commit
						41a83a7d2b
					
				| 
						 | 
					@ -294,12 +294,24 @@ static bool LineIsMarkedWithNOLINT(SourceManager& SM, SourceLocation Loc) {
 | 
				
			||||||
  return false;
 | 
					  return false;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					static bool LineIsMarkedWithNOLINTinMacro(SourceManager &SM,
 | 
				
			||||||
 | 
					                                          SourceLocation Loc) {
 | 
				
			||||||
 | 
					  while (true) {
 | 
				
			||||||
 | 
					    if (LineIsMarkedWithNOLINT(SM, Loc))
 | 
				
			||||||
 | 
					      return true;
 | 
				
			||||||
 | 
					    if (!Loc.isMacroID())
 | 
				
			||||||
 | 
					      return false;
 | 
				
			||||||
 | 
					    Loc = SM.getImmediateExpansionRange(Loc).first;
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					  return false;
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
 | 
					void ClangTidyDiagnosticConsumer::HandleDiagnostic(
 | 
				
			||||||
    DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
 | 
					    DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
 | 
				
			||||||
  if (Info.getLocation().isValid() &&
 | 
					  if (Info.getLocation().isValid() &&
 | 
				
			||||||
      DiagLevel != DiagnosticsEngine::Error &&
 | 
					      DiagLevel != DiagnosticsEngine::Error &&
 | 
				
			||||||
      DiagLevel != DiagnosticsEngine::Fatal &&
 | 
					      DiagLevel != DiagnosticsEngine::Fatal &&
 | 
				
			||||||
      LineIsMarkedWithNOLINT(Diags->getSourceManager(), Info.getLocation())) {
 | 
					      LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(), Info.getLocation())) {
 | 
				
			||||||
    ++Context.Stats.ErrorsIgnoredNOLINT;
 | 
					    ++Context.Stats.ErrorsIgnoredNOLINT;
 | 
				
			||||||
    return;
 | 
					    return;
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -13,4 +13,18 @@ void f() {
 | 
				
			||||||
  int j; // NOLINT
 | 
					  int j; // NOLINT
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
// CHECK-MESSAGES: Suppressed 3 warnings (3 NOLINT)
 | 
					#define MACRO(X) class X { X(int i); };
 | 
				
			||||||
 | 
					MACRO(D)
 | 
				
			||||||
 | 
					// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
 | 
				
			||||||
 | 
					MACRO(E) // NOLINT
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					#define MACRO_NOARG class F { F(int i); };
 | 
				
			||||||
 | 
					MACRO_NOARG // NOLINT
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					#define MACRO_NOLINT class G { G(int i); }; // NOLINT
 | 
				
			||||||
 | 
					MACRO_NOLINT
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					#define DOUBLE_MACRO MACRO(H) // NOLINT
 | 
				
			||||||
 | 
					DOUBLE_MACRO
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					// CHECK-MESSAGES: Suppressed 7 warnings (7 NOLINT)
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in New Issue