6 ms·

It is a fuck up. Ignoring a sequence point (function call) is serious business. Probably a too agressive inlining.

saagarjha 6y ago
Relax, it’s a bug in a prerelease version of a compiler that was caught early and fixed promptly. Sure, it’s a bug in an important part of the compiler, but there’s not need to call foul.
- sz4kerto 6y ago
  It might not be that simple: bugs like this should preferably be caught by the test suite of the compiler.
  - saagarjha 6y ago
    It should, but I don’t think it’s fair to say it’s a huge issue. Perhaps it’s just highlighting an area of improvement for the test suite or perhaps a need for more caution when working in that area of the compiler.
  - kryptiskt 6y ago
    Anything and everything with tests implemented in the right languages can be used as a test suite for a compiler, though. So, I'd rather see that that was done systematically than trying to catch everything with the compiler's test suite. I mean something like setting up something building and running the tests for everything included in Debian, homebrew and vcpkg and looking for regressions compared to the last release of the compiler.
  - fluffything 6y ago
    LLVM is one of the most critical pieces of infrastructure out there. And yet you can modify master without actually passing any tests. I build LLVM from source a lot, and master not even building because somebody made a change that doesn't even compiler is astonishing. FWIW, LLVM has a pretty big test suite. But... to catch errors... you actually... have to... run it... It is often the case that Rust, which does have a policy of "master always passes all tests" catches bugs in LLVM before LLVM does.
    - mehrdadn 6y ago
      Honestly "master doesn't pass tests" is just a naming problem. Solving is just a matter of renaming master to something else ('dev'? 'potentially-incorrect'?) and making a new version of it that does pass tests.
      - tom_mellior 6y ago
        That would mean that there would be a default branch with the meaning of "development branch that is as up to date as possible while still guaranteed to build and pass tests". Which would be a change from current practice, since there is currently no such branch under any name. So this is not just a naming problem. It is a problem of maintaining a development branch that is as up to date as possible while still guaranteed to build and pass tests.
        mehrdadn 6y ago
        Aren't there prerelease builds though? The branch of prereleases seems to fall under that bucket.
        tom_mellior 6y ago
        From a very quick search I can't seem to find such a branch. Judging from https://github.com/llvm/llvm-project/releases https://github.com/llvm/llvm-project/releases they seem to occasionally branch release candidate branches from master, every two weeks or so. That's very far from a well-tested master updated several times a day. Continuous integration for compilers is a well-understood problem. There are posts in this thread explaining how Rust handles it. I understand that it's a royal pain to set up and maintain. I am a compiler engineer and am happy to work on compilation stuff but wouldn't want to touch our CI system with a ten-foot pole. But LLVM is a project driven by Google and Apple, there are really no excuses for not finding the people willing to do this.
        deleted 6y ago
        [deleted]
        6y ago
- zmodem 6y ago
  Do you have a pointer to where this was fixed in the compiler?
  - saagarjha 6y ago
    I do not; I think I misread the bug report as somehow saying this had been fixed; I think I should really go to bed. I actually have no idea if this has been fixed yet, though I would still assume that it won’t be like that in master for long. Sorry for the unsubstantiated claim!
    - zmodem 6y ago
      No problem, I was just wondering if I missed something. Filed https://bugs.llvm.org/show_bug.cgi?id=46194 https://bugs.llvm.org/show_bug.cgi?id=46194
  - cesarb 6y ago
    http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200601/789353.html http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-202... says that the offending change was reverted.

Relax, it’s a bug in a prerelease version of a compiler that was caught early and fixed promptly. Sure, it’s a bug in an important part of the compiler, but there’s not need to call foul.

sz4kerto 6y ago
It might not be that simple: bugs like this should preferably be caught by the test suite of the compiler.
- saagarjha 6y ago
  It should, but I don’t think it’s fair to say it’s a huge issue. Perhaps it’s just highlighting an area of improvement for the test suite or perhaps a need for more caution when working in that area of the compiler.
- kryptiskt 6y ago
  Anything and everything with tests implemented in the right languages can be used as a test suite for a compiler, though. So, I'd rather see that that was done systematically than trying to catch everything with the compiler's test suite. I mean something like setting up something building and running the tests for everything included in Debian, homebrew and vcpkg and looking for regressions compared to the last release of the compiler.
- fluffything 6y ago
  LLVM is one of the most critical pieces of infrastructure out there. And yet you can modify master without actually passing any tests. I build LLVM from source a lot, and master not even building because somebody made a change that doesn't even compiler is astonishing. FWIW, LLVM has a pretty big test suite. But... to catch errors... you actually... have to... run it... It is often the case that Rust, which does have a policy of "master always passes all tests" catches bugs in LLVM before LLVM does.
  - mehrdadn 6y ago
    Honestly "master doesn't pass tests" is just a naming problem. Solving is just a matter of renaming master to something else ('dev'? 'potentially-incorrect'?) and making a new version of it that does pass tests.
    - tom_mellior 6y ago
      That would mean that there would be a default branch with the meaning of "development branch that is as up to date as possible while still guaranteed to build and pass tests". Which would be a change from current practice, since there is currently no such branch under any name. So this is not just a naming problem. It is a problem of maintaining a development branch that is as up to date as possible while still guaranteed to build and pass tests.
      - mehrdadn 6y ago
        Aren't there prerelease builds though? The branch of prereleases seems to fall under that bucket.
        tom_mellior 6y ago
        From a very quick search I can't seem to find such a branch. Judging from https://github.com/llvm/llvm-project/releases https://github.com/llvm/llvm-project/releases they seem to occasionally branch release candidate branches from master, every two weeks or so. That's very far from a well-tested master updated several times a day. Continuous integration for compilers is a well-understood problem. There are posts in this thread explaining how Rust handles it. I understand that it's a royal pain to set up and maintain. I am a compiler engineer and am happy to work on compilation stuff but wouldn't want to touch our CI system with a ten-foot pole. But LLVM is a project driven by Google and Apple, there are really no excuses for not finding the people willing to do this.
        deleted 6y ago
        [deleted]
        6y ago
zmodem 6y ago
Do you have a pointer to where this was fixed in the compiler?
- saagarjha 6y ago
  I do not; I think I misread the bug report as somehow saying this had been fixed; I think I should really go to bed. I actually have no idea if this has been fixed yet, though I would still assume that it won’t be like that in master for long. Sorry for the unsubstantiated claim!
  - zmodem 6y ago
    No problem, I was just wondering if I missed something. Filed https://bugs.llvm.org/show_bug.cgi?id=46194 https://bugs.llvm.org/show_bug.cgi?id=46194
- cesarb 6y ago
  http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200601/789353.html http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-202... says that the offending change was reverted.