[cfe-dev] Macros for revision number and commit hash

Nico Weber via cfe-dev cfe-dev at lists.llvm.org
Sun Mar 17 10:58:44 PDT 2019 

This would basically undo https://reviews.llvm.org/D37272 which also caused
lots of incremental build work after every sync (and technically, after
every local commit). So if this got added, it should probably be opt-in and
only be set for production builds, not for dev builds.

Since you can't compare git hashes, when do you imagine would
__clang_commit_hash__ be useful?

Doesn't __clang_revision_number__ solve the same problem that
__has_feature() and friends solve, only in a worse way?

In practice, I found all the __clang_major__ / __clang_minor__ etc
built-ins to be useless because they're gratuitously different in Xcode's
clang and there isn't a vendor id define. I suppose that's what Troy said
above.

So overall maybe this isn't fully thought through yet.

On Sun, Mar 17, 2019 at 1:02 PM Tobias Hieta via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> I am not familiar with the Clang code base, but I faced a similar issue
> recently. We made sure to only trigger a single rebuild when getting a new
> hash by generating a small CPP file that contained the hash instead of
> sticking it in a header that was included everywhere. This would only
> rebuild the generated CPP and relink. Maybe another thing that can be
> considered is to not refresh the hash when in debug configuration? This
> would allow you to build/develop locally without rebuilds when you commit
> or pull.
>
> Let me know if you want me to dig up my CMake incantation I used for this
> and I can post it to the list.
>
> Thanks,
> Tobias.
>
> > On 17 Mar 2019, at 17:54, JF Bastien via cfe-dev <cfe-dev at lists.llvm.org>
> wrote:
> >
> >
> >
> >> On Mar 17, 2019, at 9:51 AM, JF Bastien <jfbastien at apple.com> wrote:
> >>
> >>
> >>
> >>>> On Mar 17, 2019, at 7:06 AM, Roman Lebedev <lebedev.ri at gmail.com>
> wrote:
> >>>>
> >>>> On Sun, Mar 17, 2019 at 4:59 PM JF Bastien <jfbastien at apple.com>
> wrote:
> >>>>
> >>>>
> >>>>
> >>>>> On Mar 17, 2019, at 12:51 AM, Roman Lebedev <lebedev.ri at gmail.com>
> wrote:
> >>>>>
> >>>>> On Sun, Mar 17, 2019 at 6:51 AM Chris Lattner via cfe-dev
> >>>>> <cfe-dev at lists.llvm.org> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Mar 15, 2019, at 2:13 PM, JF Bastien via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> >>>>>>
> >>>>>> Hello macro enthusiasts!
> >>>>>>
> >>>>>> By default, clang defines a lot of macros that allow developers to
> figure out things about the compiler that’s building them. I’m proposing
> that we add two more:
> >>>>>>
> >>>>>> __clang_revision_number__ : an unsigned integer, representing the
> current branch’s revision number.
> >>>>>> __clang_commit_hash__ : a string containing a hexadecimal hash,
> representing the current checkout’s hash.
> >>>>>>
> >>>>>> These would be set by cmake when building LLVM, and make the most
> sense in a monorepo on git. There’s been discussion of clang revision
> number in the context of git migration, captured on the git migration page.
> >>>>>>
> >>>>>>
> >>>>>> I’m generally +1 on exposing such metadata.  The only downside I
> see is that a git pull could cause a relink of the entire world (even if
> otherwise unnecessary) just to update these fields.  I suspect that this is
> going to be more painful in a monorepo world for people who work downstream
> of clang (e.g. LLDB developers).
> >>>>>
> >>>>> That will indeed cause relinking at best, or even recompilation at
> worst.
> >>>>>
> >>>>>> Is there an implementation approach that allows us to get the
> benefit of this extra metadata without reducing dev productivity?
> >>>>>
> >>>>> The somewhat reasonable solution (apart from not doing this at all)
> >>>>> would be to build that
> >>>>> info into into *always*-shared library, so it would be the only one
> in
> >>>>> the need of relinking.
> >>>>>
> >>>>> Though i do believe from practice (from doing that on another
> project)
> >>>>> that will still
> >>>>> likely require relinking of every other lib/bin that links against
> >>>>> that version-library.
> >>>>> Not sure if that can be fixed too, would love to know how if it can.
> >>>>
> >>>> Same problem __DATE__ has. In terms of “bad things can happen” this
> is pretty low IMO.
> >>> Yes, but i'm not sure this is on the same scale of badness.
> >>>
> >>> That macro should change once a day i think, at most?
> >>> One usually commits *much* more frequently,
> >>> from 1 commit/hour to 1 commit/5minutes i'd guesstimate.
> >>
> >> You rebuild llvm and build your codebase using the new llvm that often?
> >>
> >> IMO the main source of badness from __DATE__ is the non-
> reproducibility (for which we have a warning). Here it’s already
> non-reproducible because you’ve changed your compiler... I’d certainly
> expect a full rebuild when these new macros change: you changed the
> compiler. It’s a bug if it doesn’t cause a rebuild.
> >
> > Actually I get the concern now! You mean cmake changes cause all of llvm
> to rebuild, and that would trigger on a llvm dev’s machine! Not as user
> code.
> >
> > Sorry I was slow to grok. I’ll try it out and see. Maybe incremental
> builds shouldn’t have the new commit? Or we might be able to limit the
> rebuild to Version.cpp. Or maybe the linker should do this? 🤷‍♂️
> >
> >
> >>>>>> -Chris
> >>>>> Roman.
> >>>
> >>> Roman.
> >>>
> >>>>>> Here are examples of similar numbers we currently offer:
> >>>>>>
> >>>>>> auto major     = __clang_major__;      // 7
> >>>>>> auto minor     = __clang_minor__;      // 0
> >>>>>> auto patch     = __clang_patchlevel__; // 0
> >>>>>> auto version   = __clang_version__;    // "7.0.0
> (tags/RELEASE_700/final 342594)”
> >>>>>> auto VERSION   = __VERSION__;          // "4.2.1 Compatible Clang
> 7.0.0 (tags/RELEASE_700/final 342594)”
> >>>>>> auto gnu_major = __GNUC__;             // 4
> >>>>>> auto gnu_minor = __GNUC_MINOR__;       // 2
> >>>>>> auto gnu_patch = __GNUC_PATCHLEVEL__;  // 1
> >>>>>> auto gxx_abi   = __GXX_ABI_VERSION;    // 1002
> >>>>>>
> >>>>>>
> >>>>>> The two new values will be generally useful to tools that
> automatically handle revisions. For example, we currently have bots that
> parse clang -v, and that output is unfortunately aimed at humans (bots
> aren’t human). That’s broken in the past, and having these two values would
> help reduce the likelihood of breakage because macros have a pretty stable
> format and are easy to test.
> >>>>>>
> >>>>>> Let us know what you think!
> >>>>>>
> >>>>>> JF
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> cfe-dev mailing list
> >>>>>> cfe-dev at lists.llvm.org
> >>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >>>>>>
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> cfe-dev mailing list
> >>>>>> cfe-dev at lists.llvm.org
> >>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190317/cd817061/attachment.html>

More information about the cfe-dev mailing list