[PATCH] Rename preprocessor symbols to be more descriptive. NFC

[Robinson, Paul]

Reverted in r223987.  Sorry about the data lossage!

From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Robinson, Paul
Sent: Wednesday, December 10, 2014 3:47 PM
To: Chris Matthews
Cc: Richard Smith; cfe-commits at cs.uiuc.edu
Subject: RE: [PATCH] Rename preprocessor symbols to be more descriptive. NFC

No, that was not my intent at all; it was just a "shouldn't this have a better name?" review comment.  Which I originally resisted, then gave in…
I had no idea anything outside of clang/lib/Basic/Version.cpp depended on it.
I am perfectly happy to revert r223578 and leave everything the way it was!
Gimme a second…
--paulr


From: Chris Matthews [mailto:chris.matthews at apple.com]
Sent: Wednesday, December 10, 2014 3:37 PM
To: Robinson, Paul
Cc: Richard Smith; Jordan Rose; cfe-commits at cs.uiuc.edu
Subject: Re: [PATCH] Rename preprocessor symbols to be more descriptive. NFC

Hi Paul,

This change is causing a problem with the perf data.

The bots set SVN_REVISION during builds.  LNT then uses the revision that the compiler reports to index the performance data.  With this change, all performance data is being lost (well it is all ending up as 0).  All performance data being collected since this change has been lost on all the bots.

http://llvm-lnt.herokuapp.com/db_default/v4/nts/recent_activity

We are going to need to change every bot/build to use the new name.  Was it your intention to modify the build’s interface?  If so, could you revert while we coordinate the bot updates?

Maybe making this not a silent error would help. Something like error if SVN_REVISION is passed and CLANG_REVISION is not.  Then at least we don’t have compilers with no version data encoded in them, when we expect them to.

Thanks!

On Dec 5, 2014, at 9:07 PM, Robinson, Paul <Paul_Robinson at playstation.sony.com<mailto:Paul_Robinson at playstation.sony.com>> wrote:

r223578, thanks!

From: metafoo at gmail.com<mailto:metafoo at gmail.com> [mailto:metafoo at gmail.com] On Behalf Of Richard Smith
Sent: Friday, December 05, 2014 5:15 PM
To: Jordan Rose
Cc: Robinson, Paul; cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
Subject: Re: [PATCH] Rename preprocessor symbols to be more descriptive. NFC

+1.

On Fri, Dec 5, 2014 at 4:35 PM, Jordan Rose <jordan_rose at apple.com<mailto:jordan_rose at apple.com>> wrote:
+1 from me, and I like the name choice.

> On Dec 5, 2014, at 15:41, Robinson, Paul <Paul_Robinson at playstation.sony.com<mailto:Paul_Robinson at playstation.sony.com>> wrote:
>
> This seemed a *little* too intricate to commit directly even if it is NFC.
> Also gives everyone a chance to disagree with the name choices. :-)
>
> This is a followup to a GetSVN.cmake change; Sean Silva and Jordan Rose
> asked to have the name of the SVN_REVISION/SVN_REPOSITORY symbols changed.
> This requires changing both the Makefile and CMakeLists.txt so they both
> generate the same symbol name, and Version.cpp which is the only consumer.
>
> The header generated by CMakeLists.txt/GetSVN.cmake used to look like this:
>
> #define LLVM_REVISION "223538"
> #define LLVM_REPOSITORY "http://llvm.org/svn/llvm-project/llvm/trunk"
> #define SVN_REVISION "223538"
> #define SVN_REPOSITORY "http://llvm.org/svn/llvm-project/cfe/trunk"
>
> and now looks like this:
>
> #define LLVM_REVISION "223538"
> #define LLVM_REPOSITORY "http://llvm.org/svn/llvm-project/llvm/trunk"
> #define CLANG_REVISION "223538"
> #define CLANG_REPOSITORY "http://llvm.org/svn/llvm-project/cfe/trunk"
>
> --paulr
> <svn_revision.diff>


_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141210/96a40b87/attachment.html>