Zachary Turner via llvm-dev
2017-Dec-13 01:11 UTC
[llvm-dev] [cfe-dev] Who wants faster LLVM/Clang builds?
I'm a little late to the party, but one observation that I haven't seen mentioned is that simply removing #includes and testing that the program compiles is not guaranteed to be a correct transformation. Imagine, for example, that a header file provides an overload of a function that is a better match than one found elsewhere. It will compile either way, but without the #include, you will call a different function. Note that I'm not encouraging this kind of pattern, but the point is just to illustrate that this is not necessarily a correct transformation. Another example is in the use of a macro definitions. Suppose a header defines certain macros, and other code uses ifdefs to test for the definition of those macros. If it is defined, one code path is taken, if it is not defined, another code path (which will compile but do the wrong thing) will be taken. Does the tool handle this? On Tue, Dec 12, 2017 at 1:38 PM Mikhail Zolotukhin via cfe-dev < cfe-dev at lists.llvm.org> wrote:> On Dec 12, 2017, at 12:57 PM, James Y Knight <jyknight at google.com> wrote: > > > > On Mon, Dec 11, 2017 at 3:37 PM, Mikhail Zolotukhin via cfe-dev < > cfe-dev at lists.llvm.org> wrote: > >> Hi Kim, >> >> On Dec 10, 2017, at 7:39 AM, Kim Gräsman <kim.grasman at gmail.com> wrote: >> >> Hi Michael, >> >> On Thu, Dec 7, 2017 at 3:16 AM, Michael Zolotukhin >> <mzolotukhin at apple.com> wrote: >> >> >> Nice to IWYU developers here:) I wonder how hard it would be to run IWYU >> on >> LLVM/Clang (or, if it’s supposed to work, I wonder what I did wrong). >> >> >> There are known problems with running IWYU over LLVM/Clang -- Zachary >> Turner made an attempt a while back to get it up and running. Since >> the LLVM tree uses all sorts of modern and moderately complex >> patterns, we're struggling to keep up. >> >> I see. >> >> >> If we also can tweak it a bit to make it choose more human-like (~more >> conservative) decisions, we would be able to just apply what it suggests! >> >> >> Different humans appear to have different preferences :) >> >> True, what I meant hear is to make the changes more conservative: e.g. if >> we can replace >> #include "MyClass.h" >> with >> class MyClass; >> then this change is probably desirable in every way: it documents the >> code better, it decreases coupling, it improves compile time. >> > > This is not a transform which is clearly "desirable in every way", because > it _increases_ coupling between the user of the class and the > implementation. The owner of the class can't add optional template > arguments, change a class into a typedef, change the namespace, or other > such refactorings. It also introduces the possibility of code which changes > behavior depending on whether the full or forward decl are available > (which, then, may be an ODR-violation). > > Effectively the same reasons why the standard forbids users from > forward-declaring std:: names apply to doing so to user-defined names. > > https://google.github.io/styleguide/cppguide.html#Forward_Declarations > lists some of the issues, and a recommendation not to do so. > > Of course you do have the upside is that it can improve compile time. > Which is certainly of value, and perhaps that's a worthwhile trade-off when > the implementation and forward-declare are both within a single project and > thus easy to coordinate. But, it's not by any means a _pure_ win. > > That's correct. I was speaking about the LLVM codebase though (I should've > stated that clearer), and in LLVM I don't remember many occasions of > refactorings you mentioned. For LLVM forward declaration is recommended by > the style guide: > http://llvm.org/docs/CodingStandards.html#minimal-list-of-includes > > Thanks, > Michael > > > > > _______________________________________________ > cfe-dev mailing list > cfe-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171213/bbc9d652/attachment-0001.html>
Michael Zolotukhin via llvm-dev
2017-Dec-13 01:42 UTC
[llvm-dev] [cfe-dev] Who wants faster LLVM/Clang builds?
Yes, that’s a valid concern, and that’s why I wanted to review all the changes before committing them (right now I’m going through all of them exactly to catch cases like you mention). It is actually happening sometimes, a good example is include of “config.h” - usually everything compiles even without it, but that’s not what we want. My plan is to go through all the changes, remove undesired (like the ones you mentioned) and commit remaining changes in chunks (e.g. a folder at a time). Then people can do post-commit review, while the changes will get more exposure for testing. I hope it will go smoothly, but the time will show. If there are any objections to my plan, I can put the patches for a pre-commit review first, though I consider them pretty safe. Michael> On Dec 12, 2017, at 5:11 PM, Zachary Turner <zturner at google.com> wrote: > > I'm a little late to the party, but one observation that I haven't seen mentioned is that simply removing #includes and testing that the program compiles is not guaranteed to be a correct transformation. Imagine, for example, that a header file provides an overload of a function that is a better match than one found elsewhere. It will compile either way, but without the #include, you will call a different function. Note that I'm not encouraging this kind of pattern, but the point is just to illustrate that this is not necessarily a correct transformation. > > Another example is in the use of a macro definitions. Suppose a header defines certain macros, and other code uses ifdefs to test for the definition of those macros. If it is defined, one code path is taken, if it is not defined, another code path (which will compile but do the wrong thing) will be taken. Does the tool handle this? > > > On Tue, Dec 12, 2017 at 1:38 PM Mikhail Zolotukhin via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote: >> On Dec 12, 2017, at 12:57 PM, James Y Knight <jyknight at google.com <mailto:jyknight at google.com>> wrote: >> >> >> >> On Mon, Dec 11, 2017 at 3:37 PM, Mikhail Zolotukhin via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote: >> Hi Kim, >> >>> On Dec 10, 2017, at 7:39 AM, Kim Gräsman <kim.grasman at gmail.com <mailto:kim.grasman at gmail.com>> wrote: >>> >>> Hi Michael, >>> >>> On Thu, Dec 7, 2017 at 3:16 AM, Michael Zolotukhin >>> <mzolotukhin at apple.com <mailto:mzolotukhin at apple.com>> wrote: >>>> >>>> Nice to IWYU developers here:) I wonder how hard it would be to run IWYU on >>>> LLVM/Clang (or, if it’s supposed to work, I wonder what I did wrong). >>> >>> There are known problems with running IWYU over LLVM/Clang -- Zachary >>> Turner made an attempt a while back to get it up and running. Since >>> the LLVM tree uses all sorts of modern and moderately complex >>> patterns, we're struggling to keep up. >> I see. >>> >>>> If we also can tweak it a bit to make it choose more human-like (~more >>>> conservative) decisions, we would be able to just apply what it suggests! >>> >>> Different humans appear to have different preferences :) >> True, what I meant hear is to make the changes more conservative: e.g. if we can replace >> #include "MyClass.h" >> with >> class MyClass; >> then this change is probably desirable in every way: it documents the code better, it decreases coupling, it improves compile time. >> >> This is not a transform which is clearly "desirable in every way", because it _increases_ coupling between the user of the class and the implementation. The owner of the class can't add optional template arguments, change a class into a typedef, change the namespace, or other such refactorings. It also introduces the possibility of code which changes behavior depending on whether the full or forward decl are available (which, then, may be an ODR-violation). >> >> Effectively the same reasons why the standard forbids users from forward-declaring std:: names apply to doing so to user-defined names. >> >> https://google.github.io/styleguide/cppguide.html#Forward_Declarations <https://google.github.io/styleguide/cppguide.html#Forward_Declarations> lists some of the issues, and a recommendation not to do so. >> >> Of course you do have the upside is that it can improve compile time. Which is certainly of value, and perhaps that's a worthwhile trade-off when the implementation and forward-declare are both within a single project and thus easy to coordinate. But, it's not by any means a _pure_ win. > > That's correct. I was speaking about the LLVM codebase though (I should've stated that clearer), and in LLVM I don't remember many occasions of refactorings you mentioned. For LLVM forward declaration is recommended by the style guide: > http://llvm.org/docs/CodingStandards.html#minimal-list-of-includes <http://llvm.org/docs/CodingStandards.html#minimal-list-of-includes> > > Thanks, > Michael > >> >> > > _______________________________________________ > cfe-dev mailing list > cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171212/0083fd27/attachment.html>
Zachary Turner via llvm-dev
2017-Dec-13 01:47 UTC
[llvm-dev] [cfe-dev] Who wants faster LLVM/Clang builds?
Can you just compare CRC32s of the binaries before and after your changes? And then just don’t commit unless they’re the same. You’ll have to do this with debug info disabled, as white space changes will affect debug info. That said, I’m not lgtm’ing this, just saying that if the community does decide this is a good plan, let’s make sure generated code is identical On Tue, Dec 12, 2017 at 5:42 PM Michael Zolotukhin <mzolotukhin at apple.com> wrote:> Yes, that’s a valid concern, and that’s why I wanted to review all the > changes before committing them (right now I’m going through all of them > exactly to catch cases like you mention). It is actually happening > sometimes, a good example is include of “config.h” - usually everything > compiles even without it, but that’s not what we want. > > My plan is to go through all the changes, remove undesired (like the ones > you mentioned) and commit remaining changes in chunks (e.g. a folder at a > time). Then people can do post-commit review, while the changes will get > more exposure for testing. I hope it will go smoothly, but the time will > show. > > If there are any objections to my plan, I can put the patches for a > pre-commit review first, though I consider them pretty safe. > > Michael > > On Dec 12, 2017, at 5:11 PM, Zachary Turner <zturner at google.com> wrote: > > I'm a little late to the party, but one observation that I haven't seen > mentioned is that simply removing #includes and testing that the program > compiles is not guaranteed to be a correct transformation. Imagine, for > example, that a header file provides an overload of a function that is a > better match than one found elsewhere. It will compile either way, but > without the #include, you will call a different function. Note that I'm > not encouraging this kind of pattern, but the point is just to illustrate > that this is not necessarily a correct transformation. > > Another example is in the use of a macro definitions. Suppose a header > defines certain macros, and other code uses ifdefs to test for the > definition of those macros. If it is defined, one code path is taken, if > it is not defined, another code path (which will compile but do the wrong > thing) will be taken. Does the tool handle this? > > > On Tue, Dec 12, 2017 at 1:38 PM Mikhail Zolotukhin via cfe-dev < > cfe-dev at lists.llvm.org> wrote: > >> On Dec 12, 2017, at 12:57 PM, James Y Knight <jyknight at google.com> wrote: >> >> >> >> On Mon, Dec 11, 2017 at 3:37 PM, Mikhail Zolotukhin via cfe-dev < >> cfe-dev at lists.llvm.org> wrote: >> >>> Hi Kim, >>> >>> On Dec 10, 2017, at 7:39 AM, Kim Gräsman <kim.grasman at gmail.com> wrote: >>> >>> Hi Michael, >>> >>> On Thu, Dec 7, 2017 at 3:16 AM, Michael Zolotukhin >>> <mzolotukhin at apple.com> wrote: >>> >>> >>> Nice to IWYU developers here:) I wonder how hard it would be to run IWYU >>> on >>> LLVM/Clang (or, if it’s supposed to work, I wonder what I did wrong). >>> >>> >>> There are known problems with running IWYU over LLVM/Clang -- Zachary >>> Turner made an attempt a while back to get it up and running. Since >>> the LLVM tree uses all sorts of modern and moderately complex >>> patterns, we're struggling to keep up. >>> >>> I see. >>> >>> >>> If we also can tweak it a bit to make it choose more human-like (~more >>> conservative) decisions, we would be able to just apply what it suggests! >>> >>> >>> Different humans appear to have different preferences :) >>> >>> True, what I meant hear is to make the changes more conservative: e.g. >>> if we can replace >>> #include "MyClass.h" >>> with >>> class MyClass; >>> then this change is probably desirable in every way: it documents the >>> code better, it decreases coupling, it improves compile time. >>> >> >> This is not a transform which is clearly "desirable in every way", >> because it _increases_ coupling between the user of the class and the >> implementation. The owner of the class can't add optional template >> arguments, change a class into a typedef, change the namespace, or other >> such refactorings. It also introduces the possibility of code which changes >> behavior depending on whether the full or forward decl are available >> (which, then, may be an ODR-violation). >> >> Effectively the same reasons why the standard forbids users from >> forward-declaring std:: names apply to doing so to user-defined names. >> >> https://google.github.io/styleguide/cppguide.html#Forward_Declarations >> lists some of the issues, and a recommendation not to do so. >> >> Of course you do have the upside is that it can improve compile time. >> Which is certainly of value, and perhaps that's a worthwhile trade-off when >> the implementation and forward-declare are both within a single project and >> thus easy to coordinate. But, it's not by any means a _pure_ win. >> >> That's correct. I was speaking about the LLVM codebase though (I >> should've stated that clearer), and in LLVM I don't remember many occasions >> of refactorings you mentioned. For LLVM forward declaration is recommended >> by the style guide: >> http://llvm.org/docs/CodingStandards.html#minimal-list-of-includes >> >> Thanks, >> Michael >> >> >> >> >> _______________________________________________ >> cfe-dev mailing list >> cfe-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >> > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171213/e1f454c5/attachment.html>