[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