[PATCH] Refactor bitcode reader to simplify control.

Filipe Cabecinhas filcab+llvm.phabricator at gmail.com
Wed May 27 20:53:41 PDT 2015


In http://reviews.llvm.org/D8786#179791, @kschimpf wrote:

> However, there are a couple of (bitcode binary) tests that were generated with this violation. Hence, the flag was added to fix this problem.
>
> I'm willing to remove this flag in either (1) a later review, or (2) in a later revision. However, for this review I made the issues explicit so that the problem can be seen.


Let me know which ones have bugs. They might be easy-ish to reconstruct (especially since I have some additional practice in fiddling with bc files, now (Can't promise to deal with them very quickly, though).


================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:289
@@ -251,1 +288,3 @@
+  /// \param M the module to build.
 
+  /// \param ShouldMaterializeAll true when the module should be materialized
----------------
Why the empty line?

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:301
@@ +300,3 @@
+
+  /// \brief Cheap mechanism to just extract module triple
+  /// \returns true if an error occurred.
----------------
http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
^^ This changed recently.

Omit \brief if the brief description is just a sentence. (You might want to add the '.' at the end, though)

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:789
@@ -717,3 +788,3 @@
 namespace {
-  /// @brief A class for maintaining the slot number definition
+  /// \brief A class for maintaining the slot number definition
   /// as a placeholder for the actual definition for forward constants defs.
----------------
Omit \brief.

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:803
@@ -731,3 +802,3 @@
 
-    /// @brief Methods to support type inquiry through isa, cast, and dyn_cast.
+    /// \brief Methods to support type inquiry through isa, cast, and dyn_cast.
     static bool classof(const Value *V) {
----------------
Omit \brief.

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:3254
@@ -3125,2 +3253,3 @@
     case BitstreamEntry::EndBlock:
+      setParseState(AtTopLevel);
       return std::error_code();
----------------
We're already at top level, no? (line 3234)
I might be missing something, but it looks like we're at top level, and saw an EndBlock. Shouldn't this be an error?

http://reviews.llvm.org/D8786

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list