probinson added a comment.

Minor stuff.  This solution is surprisingly simple, the main question being 
(and I don't have an answer) whether increasing the size of PresumedLoc is okay.



================
Comment at: lib/Basic/SourceManager.cpp:1460
+        FID = FileID::get(0); // contents of files referenced by #line aren't 
in
+                              // the SourceManager
+      }
----------------
The comment would work better as a proper sentence, and on the line before the 
statement.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:429
+  // all the files referred by #line. Instead the checksum remains empty,
+  // leaving to the debugger to open the file without checksum validation.
   Optional<llvm::DIFile::ChecksumKind> CSKind =
----------------
While the comment describes the desirable result, it is actually not describing 
what is happening right here.  Maybe something like this:

Compute the checksum if possible.  If the location is affected by a #line 
directive that refers to a file, PLoc will have an invalid FileID, and we will 
correctly get no checksum.


================
Comment at: test/CodeGen/debug-info-file-checksum.c:13
+// NOCHECKSUM-LABEL: !DIFile(filename: "{{.*}}code-coverage-filter2.h", 
directory: "{{[^"]*}}")
+// NOCHECKSUM-LABEL: !DIFile(filename: "{{.*}}debug-info-file-checksum.c", 
directory: "{{[^"]*}}")
+
----------------
These directives don't need the -LABEL suffix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60283/new/

https://reviews.llvm.org/D60283



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to