[PATCH] Fix for PR 21254 - Assertion in the comment parser

Dmitri Gribenko gribozavr at gmail.com
Wed Oct 15 09:09:29 PDT 2014


On Wed, Oct 15, 2014 at 4:19 PM, Dario Domizioli
<dario.domizioli at gmail.com> wrote:
> Hello cfe-dev,
>
> I have a patch to fix PR 21254 ( http://llvm.org/bugs/show_bug.cgi?id=21254
> ).

+  // We only have a limited number of bits to encode command IDs in the
+  // CommandInfo structure, so the ID numbers can potentially wrap around and

I would end the comment here.  We don't put PR numbers into the source code.

+  // cause issues and assertions. In fact, this effect caused PR 21254.
+  // This assertion will catch the case where we have created too many commands
+  // to be encoded in the number of bits in the structure.
+  assert((NextID < (1 << CommandInfo::NumCommandIDBits))
+         && "Too many commands. We have limited bits for the command ID.");

-  unsigned ID : 8;
+  /// DRY definition of the number of bits used for a command ID.
+  enum { NumCommandIDBits = 20 };

+  /// The ID of the command. Note that we can only have about a million IDs.
+  unsigned ID : NumCommandIDBits;

Please remove the second part of the comment about the million IDs --
mentioning this is not DRY, right? :)

+// CHECK:         (CXComment_InlineCommand CommandName=[dD]
RenderNormal)))] Extent=[151:1 - 151:9]

Please remove the "Extent=" part, which encodes source information.
We don't care about it in this test.

LGTM with those fixes, thank you!

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/



More information about the cfe-commits mailing list