[cfe-commits] [PATCH] Fix diagnostic reporting of previous default statement.

[Douglas Gregor]

On Dec 4, 2010, at 2:53 AM, sashan wrote:

> Hi
> 
> This is related to this thread here:
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2010-November/012302.html
> 
> This patch fixes the incorrect error reporting where the wrong previous line was
> identified as containing the duplicate 'default'. The patch also contains a test
> case for this. All the other test cases pass. Applying with:
> 
>  patch -p1 < patchfile
> 
> from the clang root source dir should work.

diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index 7ab30d1..5f07fd4 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -550,8 +550,18 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
 
     if (DefaultStmt *DS = dyn_cast<DefaultStmt>(SC)) {
       if (TheDefaultStmt) {
-        Diag(DS->getDefaultLoc(), diag::err_multiple_default_labels_defined);
-        Diag(TheDefaultStmt->getDefaultLoc(), diag::note_duplicate_case_prev);
+        PresumedLoc DSPLoc = SourceMgr.getPresumedLoc(DS->getDefaultLoc());
+        PresumedLoc TheDSPLoc = SourceMgr.getPresumedLoc(TheDefaultStmt->getDefaultLoc());
+
+        // Check which of the default statements is the duplicate by comparing
+        // their line numbers.
+        if (TheDSPLoc.getLine() < DSPLoc.getLine()) {
+          Diag(DS->getDefaultLoc(), diag::err_multiple_default_labels_defined);
+          Diag(TheDefaultStmt->getDefaultLoc(), diag::note_duplicate_case_prev);
+        } else {
+          Diag(TheDefaultStmt->getDefaultLoc(), diag::err_multiple_default_labels_defined);
+          Diag(DS->getDefaultLoc(), diag::note_duplicate_case_prev);
+        }
 

Comparing just based on the line number won't work, since the two cases could be in different files, or could appear on the same line. SourceManager::isBeforeInTranslationUnit() is the appropriate way to compare two source locations.

That said, I don't think this is the right solution to the problem. I think that, instead, we should be processing (and probably also storing) the list of case/default statements in source order rather than reverse source order, as we currently are. That would address both the problem you're seeing and the other problems I had previously alluded to, where we get diagnostics about case values in the opposite order we expect.

	- Doug