[VerifyDiagnosticConsumer] Fix last line being discarded when parsing newline

[vangthao]

Summary

In expected diagnostic checks containing newlines, anything after the last
newline would be discarded since we stop parsing directly after the last newline.
Fix this behavior so that any remaining string after the last newline is
appended instead of ignored.

[vangthao]

Ping..

[jkorous]

Thank you for the patch!

clang/test/SemaCXX/references.cpp

93

Can you please explain in detail what bug are you fixing?
In my understanding if we stop parsing after the last newline then the existing test would have failed. The difference seems to be only the white-space.
Am I missing something?

[vangthao]

clang/test/SemaCXX/references.cpp

93

In my understanding if we stop parsing after the last newline then the existing test would have failed.

The reason it does not fail is because we do not perform an exact match. We only do a partial match since contains() is used here when matching https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp#L102.

We parsed the following from the expected-warning check:

direct base 'A' is inaccessible due to ambiguity:
    struct C -> B -> A

and the actual output from clang is:

direct base 'A' is inaccessible due to ambiguity:
    struct C -> B -> A
    struct C -> A

The output from clang does contain the expected warning that we parsed but not all of it since we discarded the last line of \nstruct C -> struct A from the check. You can verify that by adding anything on this line. It will not fail the test because we are not checking for it.

clang/lib/Frontend/VerifyDiagnosticConsumer.cpp

for (ParseHelper PH(S); !PH.Done();) {
     D.DirectivePos = Pos;
     D.ContentBegin = Pos.getLocWithOffset(ContentBegin - PH.Begin);
 
     // Build directive text; convert \n to newlines.
     StringRef NewlineStr = "\\n";
     StringRef Content(ContentBegin, ContentEnd-ContentBegin);
     size_t CPos = 0;
     size_t FPos;
+    bool Append = false;
     while ((FPos = Content.find(NewlineStr, CPos)) != StringRef::npos) {
       D.Text += Content.substr(CPos, FPos-CPos);
       D.Text += '\n';
       CPos = FPos + NewlineStr.size();
+      Append = true;
     }
+    // Previous while loop did not add line after final newline. If there is
+    // more text after the newline, append it here.
+    if (Append && CPos != Content.size())
+      D.Text += Content.substr(CPos, Content.size());
+
     if (D.Text.empty())
       D.Text.assign(ContentBegin, ContentEnd);
 
     // Check that regex directives contain at least one regex.
     if (D.RegexKind && D.Text.find("{{") == StringRef::npos) {
       Diags.Report(D.ContentBegin, diag::err_verify_missing_regex) << D.Text;
       return false;
     }

clang/test/SemaCXX/references.cpp

 }
 int& not_initialized_error; // expected-error{{declaration of reference variable 'not_initialized_error' requires an initializer}}
 extern int& not_initialized_okay;
 
 class Test6 { // expected-warning{{class 'Test6' does not declare any constructor to initialize its non-modifiable members}}
   int& okay; // expected-note{{reference member 'okay' will never be initialized}}
 };
 
-struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due to ambiguity:\n    struct C -> struct B -> struct A\nstruct C -> struct A}}
+struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due to ambiguity:\n    struct C -> struct B -> struct A\n    struct C -> struct A}}
 
 void test7(C& c) {
   A& a1 = c; // expected-error {{ambiguous conversion from derived class 'C' to base class 'A':}}
 }
 
 // C++ [dcl.ref]p1, C++ [dcl.ref]p4
 void test8(int& const,// expected-error{{'const' qualifier may not be applied to a reference}}