LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 33507 - Assertion failed: Shift >= 0, file C:\src\llvm_package_303050\llvm\tools\clang\lib\Format\WhitespaceManager.cpp, line 245
Summary: Assertion failed: Shift >= 0, file C:\src\llvm_package_303050\llvm\tools\clan...
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: Formatter (show other bugs)
Version: trunk
Hardware: PC Windows NT
: P normal
Assignee: Beren Minor
URL:
Keywords:
: 34071 (view as bug list)
Depends on:
Blocks: 33849
  Show dependency tree
 
Reported: 2017-06-19 07:03 PDT by Teodor MICU
Modified: 2017-08-25 13:31 PDT (History)
8 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Teodor MICU 2017-06-19 07:03:27 PDT
We have a dozen of C++ files with some lambdas that make clang-format crash.

One example is this code:

  auto found = range::find_if(vsProducts, [&](auto * aProduct) {
    static const Version verVs2017;
    return true;
  });

If I add this code to another C++ file and try to format (on save), it will crash and create a dump file.  (the example code was reduced, but still two lines)

I've trimmed the config file to just these two lines to reproduce the problem:

  BasedOnStyle: Mozilla
  AlignConsecutiveDeclarations: 'true'

Thanks
Comment 1 Hans Wennborg 2017-08-08 15:53:59 PDT
I can reproduce with trunk and 5.0 but not 4.0, so this is a regression.

Daniel: is there someone besides yourself who like looking into clang-format bugs? :-)
Comment 2 Hans Wennborg 2017-08-10 14:46:02 PDT
*** Bug 34071 has been marked as a duplicate of this bug. ***
Comment 3 Andrey Rusanov 2017-08-11 03:40:44 PDT
I have made an workaround for me - simply commented out this assert. And it looks quite normal. I don't know why. :)
Comment 4 Teodor MICU 2017-08-22 04:02:02 PDT
Some *blame* history:

1) assert added in rev 248999
[clang-format] Add support of consecutive declarations alignment

2) rev 295586 the assert was still valid

3) rev 306282 the assert is NOT valid anymore

This means only four revisions left, most likely rev *298574* by nikola:
> Fix issues in clang-format's AlignConsecutive modes.

Indeed, a quick check confirms that rev 5.0.0.300231 is also affected.
Comment 5 Hans Wennborg 2017-08-22 09:24:55 PDT
+Nikola
Comment 6 Hans Wennborg 2017-08-23 08:40:49 PDT
krasimir: Maybe you could take a look?
Comment 7 Hans Wennborg 2017-08-24 11:46:18 PDT
(In reply to Andrey Rusanov from comment #3)
> I have made an workaround for me - simply commented out this assert. And it
> looks quite normal. I don't know why. :)

It seems to misformat the test case if I do that:


$ cat | bin/clang-format -style="{BasedOnStyle: Mozilla,AlignConsecutiveDeclarations: 'true'}"
auto found = range::find_if(vsProducts, [&](auto * aProduct) {
  static const Version verVs2017;
  return true;
});
auto found = range::find_if(vsProducts, [&](auto* aProduct) {
  static const VersionverVs2017;
  return true;
});


Note that it breaks the code by concatenating "Version" and "verVs2017" :-(
Comment 8 Hans Wennborg 2017-08-24 11:54:22 PDT
(In reply to Hans Wennborg from comment #7)
> Note that it breaks the code by concatenating "Version" and "verVs2017" :-(

Confirmed that both the assert and the concatenation started with r298574.
Comment 9 Beren Minor 2017-08-25 03:52:15 PDT
Hello, I've sent this patch for review if anyone wants to give it a try: https://reviews.llvm.org/D37137
Comment 10 Don Hinton 2017-08-25 05:51:24 PDT
Not sure exactly how to fix it, but here's a minimal example that asserts:

echo "v f([]() {y x; x;});" | bin/clang-format -style="{AlignConsecutiveDeclarations: 'true'}"

And, interestingly, the following doesn't assert, but as you can see, the formatting is a bit odd:

echo "vvvvvvvvvvvv f([]() {y x; x;});" | bin/clang-format -style="{AlignConsecutiveDeclarations: 'true'}"
vvvvvvvvvvvv f([]() {
  y          x;
  x;
});
Comment 11 Don Hinton 2017-08-25 05:58:04 PDT
Sorry, didn't see your post/patch before I commented, but I tested it with my test case and it works great...
Comment 12 Hans Wennborg 2017-08-25 12:58:33 PDT
The patch was committed in r311792. I'll let it bake a little bit and then merge.
Comment 13 Hans Wennborg 2017-08-25 13:31:12 PDT
Merged in r311800. Thanks for fixing!