[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina abandoned this revision.
kristina added a comment.

Superseded by D61756 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina commandeered this revision.
kristina added a reviewer: weimingz.
kristina added a comment.

Sorry, forgot about this, will make a new diff with just the macro for review 
later tonight.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nolan O'Brien via Phabricator via cfe-commits]

NSProgrammer added a comment.

In D17741#1413864 , @kristina wrote:

> If the author is still missing at the end of next week, any objections to me 
> resubmitting a similar patch that just implements `__FILE_NAME__` or 
> `__BASE_NAME__` (Need a few more opinions here I guess, personally I think 
> `__FILE_NAME__` makes more sense)?
>
> I'll carve it out from my PP extension which simply looks for the last path 
> separator (depending on the OS) and only renders the filename after it (or 
> the whole path if there's no separator). No need for additional complications 
> like depths etc. Since this idea was shot down last time, is it possible to 
> get a few people to voice their opinion before I mark this as abandoned and 
> carve out and clean up this from my PP extension and add proper tests for it?
>
> Would be appreciated, as this sort of thing is very useful (IMO) so would 
> like to know if anyone is really against this proposal.


Kristina, it looks like there is no push back and even plenty of support.  How 
do you feel about going forward with submitting a patch that implements 
`__FILE_NAME__`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

If the author is still missing at the end of next week, any objections to me 
resubmitting a similar patch that just implements `__FILE_NAME__` or 
`__BASE_NAME__` (Need a few more opinions here I guess, personally I think 
`__FILE_NAME__` makes more sense)?

I'll carve it out from my PP extension which simply looks for the last path 
separator (depending on the OS) and only renders the filename after it (or the 
whole path if there's no separator). No need for additional complications like 
depths etc. Since this idea was shot down last time, is it possible to get a 
few people to voice their opinion before I mark this as abandoned and carve out 
and clean up this from my PP extension and add proper tests for it?

Would be appreciated, as this sort of thing is very useful (IMO) so would like 
to know if anyone is really against this proposal.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nolan O'Brien via Phabricator via cfe-commits]

NSProgrammer added a comment.

We would prefer a macro like `__FILE_NAME__` over a build flag for code reading 
consistency (they would clearly do different things vs varying based on an 
obscure flag being present/absent).

This patch is languishing, unless the original author thinks otherwise, a new 
patch to push this through would be great.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

Ping for author? This is one of the changes I'd like to see through, though the 
complexity here can be massively reduced as stated above. I wouldn't mind 
opening a new diff if the author is away with just a change in PPMacroExpansion 
and a testcase (which should probably be included here) but I would appreciate 
some form of consensus since this exact idea with a special macro for filename 
was previously rejected.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Joerg Sonnenberger via cfe-commits]

On Thu, Jan 03, 2019 at 05:01:00PM +, Ximin Luo via cfe-commits wrote:
> My patch that Roman quoted in the email below was an additional
> mechanism to set -fmacro-prefix-map and -fdebug-prefix-map via an
> environment variable. This was not accepted into GCC because they are
> (in summary) allergic against using environment variables to set anything.

The main problem here is that GCC insists on leaking the full command
line into the output and that makes it difficult to impossible to
actually provide sane mappings without using the environment.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

I should add that this is not just about reproducible builds but about code 
size as mentioned by @NSProgrammer. There are ways to cheat with builtins but 
the problem is, entire __FILE__ string is still emitted into read-only data, 
despite the constexpressiveness of the value. The common way of getting the 
short filename in code would the the following (the `#else` case) which is a 
constant evaluated but has a side effect of dumping the full paths in rodata 
regardless (ICF seems unable to clean them up even in full LTO builds).

  #ifdef __clang_extension_generate
  #define OS_CUR_FILENAME  __generate("file!0")
  #else
  #define OS_CUR_FILENAME (__builtin_strrchr(__FILE__, '/') ? \
  __builtin_strrchr(__FILE__, '/') + 1 : __FILE__)
  #endif

The extension on the other hand avoids that. So it's a win for code size and 
reproducible builds. I would urge reconsidering something like `__FILE_NAME__`, 
or whatever name is preferable for just getting the last path component at 
preprocessor stage, despite it being poorly received by other developers the 
last time it seems to be a feature wanted by consumers. I would urge 
maintainers to reply to get another consensus regarding this since my stance is 
still the same, if a "missing" feature is widely hacked around, it's clearly 
desirable. While I understand that a lot have an aversion to nonstandard 
Clang-specific extensions, as long as a feature check is available it should be 
down to the consumer to decide if they want to make use of such extensions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Ximin Luo via cfe-commits]

Hi all, more strict interpretations of reproducible builds want build paths to 
not appear in the final output *regardless of the build location* and so 
/tmp/build would not be appropriate. Rough justification is that we want 
different people building under different starting conditions to be able 
reproduce identical final binaries, this is a *stronger* property than 
reproduction under *same* starting conditions.

I believe newer versions of GCC have an existing flag called -fmacro-prefix-map:

https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html

My patch that Roman quoted in the email below was an additional mechanism to 
set -fmacro-prefix-map and -fdebug-prefix-map via an environment variable. This 
was not accepted into GCC because they are (in summary) allergic against using 
environment variables to set anything.

I agree envvars are not great in general, but think an exception in this case 
was appropriate. It is very common for higher-level build scripts to store 
command-line options in various parts of the build output (making it again 
dependent on the build path, since the argument to -fmacro-prefix-map contains 
the build path). By contrast, it is almost unheard-of for tools to store 
arbitrary environment variables in the build output, and that usually indicates 
a bug.

This is why we the reproducible builds project defined some standardised 
environment variables for these purposes:

https://reproducible-builds.org/specs/source-date-epoch/
https://reproducible-builds.org/specs/build-path-prefix-map/

Inconsistently, GCC accepted our (The Reproducible Builds Project's) patch for 
the former but not the latter, due to different reviewers. (FWIW, the reviewer 
that rejected the latter patch stated that he would not have accepted the 
former patch.)

Ed Maste (added to CC) had previously tried to get SOURCE_DATE_EPOCH accepted 
into LLVM but the effort seems to have stalled.

https://reviews.llvm.org/D20791
https://reviews.llvm.org/D23934

I'll leave it to you guys to decide where to go from here.

X

Roman Lebedev:
> No, that is the opposite of the solution, actually, i think.
> https://tests.reproducible-builds.org/debian/issues/unstable/gcc_captures_build_path_issue.html
> 
> And:
> https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00513.html
> ^ i suspect clang should be compatible with whatever approach is
> suggested there,
> though i have not looked what it is. (cc'd the author of that patch here)
> 
> On Thu, Jan 3, 2019 at 6:54 PM Nolan O'Brien  wrote:
>>
>> I believe reproducible build projects simply move their source to 
>> `/tmp/build` or something else that’s consistent to deal with `__FILE__` 
>> currently.  They’d probably appreciate this to avoid file copying during 
>> `make` builds
>>
>> Nolan O'Brien
>> Sent from my iPhone
>>
>>> On Jan 3, 2019, at 7:47 AM, Roman Lebedev  wrote:
>>>
>>> Just a thought: reproducible builds will surely be interested in this.
>>> Did gcc already solve this problem? And if yes, it would be best to be
>>> compatible.
>>>
>>> On Thu, Jan 3, 2019 at 6:41 PM Nolan O'Brien via Phabricator via
>>> cfe-commits  wrote:

 NSProgrammer added a comment.

 To throw in my 2 cents.  I don’t really have a preference between a 
 compiler flag vs a different macro that’s just for the file name sans path 
 prefix.  But I have a real need for this to get into clang:  with 1.2 
 million lines of code, the regular placement of log statements and custom 
 asserts leads to megabytes in binary size from all the __FILE__ usages, 
 and that could easily be a few hundred KB with this kind of support in 
 clang.


 CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

 https://reviews.llvm.org/D17741



 ___
 cfe-commits mailing list
 cfe-commits@lists.llvm.org
 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nolan O'Brien via cfe-commits]

I believe reproducible build projects simply move their source to `/tmp/build` 
or something else that’s consistent to deal with `__FILE__` currently.  They’d 
probably appreciate this to avoid file copying during `make` builds

Nolan O'Brien
Sent from my iPhone

> On Jan 3, 2019, at 7:47 AM, Roman Lebedev  wrote:
> 
> Just a thought: reproducible builds will surely be interested in this.
> Did gcc already solve this problem? And if yes, it would be best to be
> compatible.
> 
> On Thu, Jan 3, 2019 at 6:41 PM Nolan O'Brien via Phabricator via
> cfe-commits  wrote:
>> 
>> NSProgrammer added a comment.
>> 
>> To throw in my 2 cents.  I don’t really have a preference between a compiler 
>> flag vs a different macro that’s just for the file name sans path prefix.  
>> But I have a real need for this to get into clang:  with 1.2 million lines 
>> of code, the regular placement of log statements and custom asserts leads to 
>> megabytes in binary size from all the __FILE__ usages, and that could easily 
>> be a few hundred KB with this kind of support in clang.
>> 
>> 
>> CHANGES SINCE LAST ACTION
>>  https://reviews.llvm.org/D17741/new/
>> 
>> https://reviews.llvm.org/D17741
>> 
>> 
>> 
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

In D17741#1345171 , @NSProgrammer 
wrote:

> To throw in my 2 cents.  I don’t really have a preference between a compiler 
> flag vs a different macro that’s just for the file name sans path prefix.  
> But I have a real need for this to get into clang:  with 1.2 million lines of 
> code, the regular placement of log statements and custom asserts leads to 
> megabytes in binary size from all the __FILE__ usages, and that could easily 
> be a few hundred KB with this kind of support in clang.


Yeah that would only require adding a new macro in 
`lib/Lex/PPMacroExpansion.cpp` plus a test instead of a huge diff as it is now. 
So yes as I said, that would be a great patch. However this implementation is 
is needlessly complicated and I don't think this should be yet another 
driver/compiler flag for a seemingly obscure use case. So yes I'm with you on 
that, filename without the path prefix is definitely an extension I'd like to 
see upstreamed (I proposed it on `cfe-dev` a while ago but it wasn't well 
received so I never put it up for review (that was before the weird 
preprocessor stuff that I do now in my fork)).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Roman Lebedev via cfe-commits]

No, that is the opposite of the solution, actually, i think.
https://tests.reproducible-builds.org/debian/issues/unstable/gcc_captures_build_path_issue.html

And:
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00513.html
^ i suspect clang should be compatible with whatever approach is
suggested there,
though i have not looked what it is. (cc'd the author of that patch here)

On Thu, Jan 3, 2019 at 6:54 PM Nolan O'Brien  wrote:
>
> I believe reproducible build projects simply move their source to 
> `/tmp/build` or something else that’s consistent to deal with `__FILE__` 
> currently.  They’d probably appreciate this to avoid file copying during 
> `make` builds
>
> Nolan O'Brien
> Sent from my iPhone
>
> > On Jan 3, 2019, at 7:47 AM, Roman Lebedev  wrote:
> >
> > Just a thought: reproducible builds will surely be interested in this.
> > Did gcc already solve this problem? And if yes, it would be best to be
> > compatible.
> >
> > On Thu, Jan 3, 2019 at 6:41 PM Nolan O'Brien via Phabricator via
> > cfe-commits  wrote:
> >>
> >> NSProgrammer added a comment.
> >>
> >> To throw in my 2 cents.  I don’t really have a preference between a 
> >> compiler flag vs a different macro that’s just for the file name sans path 
> >> prefix.  But I have a real need for this to get into clang:  with 1.2 
> >> million lines of code, the regular placement of log statements and custom 
> >> asserts leads to megabytes in binary size from all the __FILE__ usages, 
> >> and that could easily be a few hundred KB with this kind of support in 
> >> clang.
> >>
> >>
> >> CHANGES SINCE LAST ACTION
> >>  https://reviews.llvm.org/D17741/new/
> >>
> >> https://reviews.llvm.org/D17741
> >>
> >>
> >>
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Roman Lebedev via cfe-commits]

Just a thought: reproducible builds will surely be interested in this.
Did gcc already solve this problem? And if yes, it would be best to be
compatible.

On Thu, Jan 3, 2019 at 6:41 PM Nolan O'Brien via Phabricator via
cfe-commits  wrote:
>
> NSProgrammer added a comment.
>
> To throw in my 2 cents.  I don’t really have a preference between a compiler 
> flag vs a different macro that’s just for the file name sans path prefix.  
> But I have a real need for this to get into clang:  with 1.2 million lines of 
> code, the regular placement of log statements and custom asserts leads to 
> megabytes in binary size from all the __FILE__ usages, and that could easily 
> be a few hundred KB with this kind of support in clang.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D17741/new/
>
> https://reviews.llvm.org/D17741
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nolan O'Brien via Phabricator via cfe-commits]

NSProgrammer added a comment.

To throw in my 2 cents.  I don’t really have a preference between a compiler 
flag vs a different macro that’s just for the file name sans path prefix.  But 
I have a real need for this to get into clang:  with 1.2 million lines of code, 
the regular placement of log statements and custom asserts leads to megabytes 
in binary size from all the __FILE__ usages, and that could easily be a few 
hundred KB with this kind of support in clang.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

I like the idea of just getting the filename reliably, but I think an extension 
to strip path prefixes is a bit of an overkill especially as yet another 
compiler flag. I implemented a similar thing in my fork in form of a directive 
to `__generate(...)` which is my own weird extension to do meta-programming 
using the preprocessor, that's the implementation with no extra parameters 
(just as an idea, I know the implementation is not great):

  else if (PS == kMPS_DirectiveFile)
   {
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
 
 OutBuffer.clear();
 OutBuffer.push_back('"');
 
 if (PLoc.isValid()) {
   TokenBuffer.clear();
   TokenBuffer.append(PLoc.getFilename());
   
   size_t LastSlashPos = TokenBuffer.find_last_of('/');
   if (LastSlashPos != StringRef::npos) {
 std::string LastPathComponent = 
   Lexer::Stringify(TokenBuffer.substr(LastSlashPos+1));
 
 OutBuffer.append(LastPathComponent);
   }
   else {
 Lexer::Stringify(TokenBuffer);
 OutBuffer.append(TokenBuffer.str());
   }
 }
 else {
   OutBuffer.push_back('?');
 }
   }

I think this type of approach (I don't mean the PP extension, just the 
methodology) for getting the filename without the full path or shedding parts 
of the full path is a lot nicer. I'm not entirely sure why so many headers are 
getting pulled in either, though, I think the only issue is the path 
separator?. Aside from that I don't think this needs a compiler flag, but a 
macro for just getting the filename reliably in upstream would be nice. Your 
flag idea means various build systems may have to do gymnastics to find out the 
right path to strip so it seems cumbersome.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Karl-Johan Karlsson via Phabricator via cfe-commits]

Ka-Ka added a comment.

Ping?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Karl-Johan Karlsson via Phabricator via cfe-commits]

Ka-Ka added a comment.

Ping?

Do you think this small lit testcase below work as a testcase for the 
-ffile-macro-prefix-to-remove option?

// RUN: %clang_cc1 -emit-llvm %s  -o - -ffile-macro-prefix-to-remove=%s | 
FileCheck %s
char this_file[] = __FILE__;
// CHECK: @this_file = global [1 x i8] zeroinitializer


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a reviewer: vsapsai.
dexonsmith added a comment.

In https://reviews.llvm.org/D17741#1067816, @weimingz wrote:

> split the original into two parts. This one supports 
> -ffile-macro-prefix-to-remove function.
>
> I locally verified it.  To add a test case, do we need to use VFS?


VFS might work.


https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via Phabricator via cfe-commits]

weimingz updated this revision to Diff 142499.
weimingz edited the summary of this revision.
weimingz added a comment.

split the original into two parts. This one supports 
-ffile-macro-prefix-to-remove function.

I locally verified it.  To add a test case, do we need to use VFS?


https://reviews.llvm.org/D17741

Files:
  include/clang/Driver/Options.td
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp

Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -26,25 +26,27 @@
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/PTHLexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorLexer.h"
-#include "clang/Lex/PTHLexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -1717,7 +1719,16 @@
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
-  FN += PLoc.getFilename();
+  StringRef Filename = PLoc.getFilename();
+  if (II == Ident__FILE__) {
+const std::string  =
+getPreprocessorOpts().FileMacroPrefixToRemove;
+if (Filename.startswith(PrefixToRemove))
+  FN += Filename.substr(PrefixToRemove.size());
+else
+  FN += Filename;
+  } else
+FN += Filename;
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2826,6 +2826,10 @@
   Opts.ObjCXXARCStandardLibrary = (ObjCXXARCStandardLibraryKind)Library;
   }
 
+  if (Arg *A = Args.getLastArg(OPT_ffile_macro_prefix_to_remove)) {
+Opts.FileMacroPrefixToRemove = A->getValue();
+  }
+
   // Always avoid lexing editor placeholders when we're just running the
   // preprocessor as we never want to emit the
   // "editor placeholder in source file" error in PP only mode.
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1264,6 +1264,15 @@
 // For IAMCU add special include arguments.
 getToolChain().AddIAMCUIncludeArgs(Args, CmdArgs);
   }
+
+  // Add FILE macro prefix removing arguments or basename only flags.
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_ffile_macro_prefix_to_remove)) {
+StringRef PrefixToRemove(A->getValue());
+if (!PrefixToRemove.empty())
+  CmdArgs.push_back(Args.MakeArgString("-ffile-macro-prefix-to-remove=" +
+   PrefixToRemove));
+  }
 }
 
 // FIXME: Move to target hook.
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -127,12 +127,16 @@
   /// manipulation of the compiler invocation object, in cases where the 
   /// compiler invocation and its buffers will be reused.
   bool RetainRemappedFileBuffers = false;
-  
+
   /// \brief The Objective-C++ ARC standard library that we should support,
   /// by providing appropriate definitions to retrofit the standard library
   /// with support for lifetime-qualified pointers.
   ObjCXXARCStandardLibraryKind ObjCXXARCStandardLibrary = ARCXX_nolib;
-
+
+  /// \brief This string will be used in expanding __FILE__ macro. If it
+  /// matches the prefix of __FILE__, the matched part won't be expanded.
+  std::string FileMacroPrefixToRemove;
+
   /// \brief Records the set of modules
   class FailedModulesSet {
 llvm::StringSet<> Failed;
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1246,6 +1246,9 @@
   Flag <["-"], "fno-implicit-modules">,
   Group, Flags<[DriverOption, CC1Option]>;
 def fretain_comments_from_system_headers : Flag<["-"], "fretain-comments-from-system-headers">, Group, 

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Rick Mann via Phabricator via cfe-commits]

JetForMe added a comment.

Just want to comment that this functionality is also good for security, as it 
enables hiding of path information in final builds (while still allowing 
logging to show file and line number).


https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.

Hal requested splitting the patch in two, since the two features are separable, 
but they both still seem to be here.  Perhaps start with the prefix patch?




Comment at: include/clang/Driver/Options.td:828
 
+def FILE_prefix_to_remove : Joined<["-"], "ffile-macro-prefix-to-remove=">,
+  Group, Flags<[DriverOption, CC1Option]>,

This opt name doesn’t match the command line option. 



Comment at: include/clang/Driver/Options.td:831
+  HelpText<"prefix to be removed from expanded string of __FILE__ macro">;
+def FILE_basename_only : Flag <["-"], "ffile-macro-basename-only">,
+  Group, Flags<[DriverOption, CC1Option]>,

Same here. 



Comment at: include/clang/Lex/PreprocessorOptions.h:122
+  /// matches the prefix of __FILE__, the matched part won't be expanded.
+  std::string __FILE__PrefixToRemove;
+

This identifier name is reserved for the implementation.



Comment at: include/clang/Lex/PreprocessorOptions.h:126
+  /// basename part of __FILE__ will be expanded.
+  bool __FILE__BasenameOnly;
+

Same here. 



Comment at: lib/Lex/PPMacroExpansion.cpp:1608
 if (PLoc.isValid()) {
-  FN += PLoc.getFilename();
+  StringRef Filename(PLoc.getFilename());
+  if (II == Ident__FILE__) {

Why not use =?


https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz added a comment.

ping?


https://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz added a comment.

Ping ?


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz updated this revision to Diff 53105.
weimingz added a comment.

per Alex's suggestion, split into 2 flags:
-ffile-macro-prefix-to-remove=xxx : remove matched prefix
-ffile-macro-basename-only  : remove the whole dir part


http://reviews.llvm.org/D17741

Files:
  include/clang/Driver/Options.td
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp

Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -12,7 +12,6 @@
 //
 //===--===//
 
-#include "clang/Lex/Preprocessor.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -22,12 +21,15 @@
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -1595,7 +1597,7 @@
 PLoc = SourceMgr.getPresumedLoc(NextLoc);
 if (PLoc.isInvalid())
   break;
-
+
 NextLoc = PLoc.getIncludeLoc();
   }
 }
@@ -1603,7 +1605,18 @@
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
-  FN += PLoc.getFilename();
+  StringRef Filename(PLoc.getFilename());
+  if (II == Ident__FILE__) {
+const std::string  =
+getPreprocessorOpts().__FILE__PrefixToRemove;
+if (getPreprocessorOpts().__FILE__BasenameOnly)
+  FN += llvm::sys::path::filename(Filename);
+else if (!PrefixToRemove.empty() && Filename.startswith(PrefixToRemove))
+  FN += Filename.substr(PrefixToRemove.size());
+else
+  FN += Filename;
+  } else
+FN += Filename;
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2010,6 +2010,14 @@
 else
   Opts.ObjCXXARCStandardLibrary = (ObjCXXARCStandardLibraryKind)Library;
   }
+
+  if (Arg *A =
+  Args.getLastArg(OPT_FILE_prefix_to_remove, OPT_FILE_basename_only)) {
+if (A->getOption().matches(options::OPT_FILE_basename_only))
+  Opts.__FILE__BasenameOnly = true;
+else
+  Opts.__FILE__PrefixToRemove = A->getValue();
+  }
 }
 
 static void ParsePreprocessorOutputArgs(PreprocessorOutputOptions ,
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -570,6 +570,16 @@
   // Add CUDA include arguments, if needed.
   if (types::isCuda(Inputs[0].getType()))
 getToolChain().AddCudaIncludeArgs(Args, CmdArgs);
+
+  // Add FILE macro prefix removing arguments or basename only flags.
+  if (const Arg *A = Args.getLastArg(options::OPT_FILE_prefix_to_remove,
+ options::OPT_FILE_basename_only)) {
+if (A->getOption().matches(options::OPT_FILE_basename_only))
+  CmdArgs.push_back(Args.MakeArgString("-ffile-macro-basename-only"));
+else if (!StringRef(A->getValue()).empty())
+  CmdArgs.push_back(Args.MakeArgString(
+  Twine("-ffile-macro-prefix-to-remove=") + A->getValue()));
+  }
 }
 
 // FIXME: Move to target hook.
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -108,15 +108,23 @@
   /// the buffers associated with remapped files.
   ///
   /// This flag defaults to false; it can be set true only through direct
-  /// manipulation of the compiler invocation object, in cases where the 
+  /// manipulation of the compiler invocation object, in cases where the
   /// compiler invocation and its buffers will be reused.
   bool RetainRemappedFileBuffers;
-  
+
   /// \brief The Objective-C++ ARC standard library that we should support,
   /// by providing appropriate definitions to retrofit the standard library
   /// with support for lifetime-qualified pointers.
   ObjCXXARCStandardLibraryKind ObjCXXARCStandardLibrary;
-
+
+  /// \brief This string will be used in expanding __FILE__ macro. If it
+  /// matches the prefix of __FILE__, the matched part won't be expanded.
+  std::string __FILE__PrefixToRemove;
+
+  /// \brief This falgs defaults to false; If it is set to true, only the
+  /// 

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz added a subscriber: weimingz.
weimingz added a comment.

Sounds good. Will do.

Thanks for reviewing


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Zhao, Weiming via cfe-commits]

Sounds good. Will do.

Thanks for reviewing

On 4/8/2016 11:46 AM, Alex Rosenberg wrote:

alexr added a subscriber: alexr.
alexr added a comment.

There are multiple features in this patch that should be considered separately. 
Please split the patch.

That said, I don't think we want to add a fundamental new preprocessor feature 
like __FILE_BASENAME__ without at least getting some early buy-in from the WG14 
and WG21 committees. I suspect they’d not want to add to the preprocessor at 
all.

The flag concept seems much more tractable. Please split it into two flags, one 
to force __FILE__ to only be the basename, and one that handles the prefix 
stripping. That is, a magic value of __ALL_DIR__ seems like the wrong choice 
here.


http://reviews.llvm.org/D17741





--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by 
The Linux Foundation

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Alex Rosenberg via cfe-commits]

alexr added a subscriber: alexr.
alexr added a comment.

There are multiple features in this patch that should be considered separately. 
Please split the patch.

That said, I don't think we want to add a fundamental new preprocessor feature 
like __FILE_BASENAME__ without at least getting some early buy-in from the WG14 
and WG21 committees. I suspect they’d not want to add to the preprocessor at 
all.

The flag concept seems much more tractable. Please split it into two flags, one 
to force __FILE__ to only be the basename, and one that handles the prefix 
stripping. That is, a magic value of __ALL_DIR__ seems like the wrong choice 
here.


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz added a comment.

In http://reviews.llvm.org/D17741#374725, @weimingz wrote:

> In http://reviews.llvm.org/D17741#372098, @weimingz wrote:
>
> > rebased
>
>
> ping~


HI,

Any comments/suggestions?


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz added a comment.

In http://reviews.llvm.org/D17741#372098, @weimingz wrote:

> rebased


ping~


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz updated this revision to Diff 50300.
weimingz added a comment.

rebased


http://reviews.llvm.org/D17741

Files:
  include/clang/Driver/Options.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp
  test/Lexer/file_basename.c

Index: test/Lexer/file_basename.c
===
--- /dev/null
+++ test/Lexer/file_basename.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -E %s | FileCheck %s --check-prefix=ALL --check-prefix=DEFAULT
+
+// RUN: %clang_cc1 -E -ffile-macro-prefix-to-remove=__ALL_DIR__ %s | FileCheck %s --check-prefix=ALL --check-prefix=NO-DIR
+// RUN:%clang -E -ffile-macro-prefix-to-remove=__ALL_DIR__ %s | FileCheck %s --check-prefix=ALL --check-prefix=NO-DIR
+
+// RUN: %clang_cc1 -E -ffile-macro-prefix-to-remove=%s %s | FileCheck %s --check-prefix=ALL --check-prefix=REMOVE-ALL
+// RUN: %clang -E -ffile-macro-prefix-to-remove=%s %s | FileCheck %s --check-prefix=ALL --check-prefix=REMOVE-ALL
+
+// RUN: %clang_cc1 -E -ffile-macro-prefix-to-remove=%s.xyz %s | FileCheck %s --check-prefix=ALL --check-prefix=MISMATCH
+// RUN: %clang -E -ffile-macro-prefix-to-remove= %s | FileCheck %s --check-prefix=ALL --check-prefix=MISMATCH
+
+const char *filename (const char *name) {
+  // ALL:  static const char this_file_basename[] = "file_basename.c";
+  static const char this_file_basename[] = __CLANG_FILE_BASENAME__;
+  // DEFAULT: static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  // NO-DIR:  static const char this_file[] = "file_basename.c";
+  // REMOVE-ALL: static const char this_file[] = "";
+  // MISMATCH: static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  static const char this_file[] = __FILE__;
+  return this_file;
+}
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -12,7 +12,6 @@
 //
 //===--===//
 
-#include "clang/Lex/Preprocessor.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -22,12 +21,15 @@
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -292,6 +294,8 @@
 void Preprocessor::RegisterBuiltinMacros() {
   Ident__LINE__ = RegisterBuiltinMacro(*this, "__LINE__");
   Ident__FILE__ = RegisterBuiltinMacro(*this, "__FILE__");
+  Ident__CLANG_FILE_BASENAME__ =
+  RegisterBuiltinMacro(*this, "__CLANG_FILE_BASENAME__");
   Ident__DATE__ = RegisterBuiltinMacro(*this, "__DATE__");
   Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
   Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
@@ -1509,7 +1513,8 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__ ||
+ II == Ident__CLANG_FILE_BASENAME__) {
 // C99 6.10.8: "__FILE__: The presumed name of the current source file (a
 // character string literal)". This can be affected by #line.
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
@@ -1522,15 +1527,30 @@
 PLoc = SourceMgr.getPresumedLoc(NextLoc);
 if (PLoc.isInvalid())
   break;
-
 NextLoc = PLoc.getIncludeLoc();
   }
 }
 
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
-  FN += PLoc.getFilename();
+  const std::string  =
+  getPreprocessorOpts().__FILE__PrefixToRemove;
+  StringRef Filename(PLoc.getFilename());
+
+  if (II == Ident__CLANG_FILE_BASENAME__)
+FN += llvm::sys::path::filename(Filename);
+  else if (II == Ident__FILE__ && !PrefixToRemove.empty()) {
+if (PrefixToRemove == "__ALL_DIR__") {
+  FN += llvm::sys::path::filename(Filename);
+}
+else if (Filename.find(PrefixToRemove) == 0)
+  FN += Filename.substr(PrefixToRemove.size());
+else
+  FN += Filename;
+  } else
+FN += Filename;
+
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ 

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Craig, Ben via cfe-commits]

On 3/4/2016 6:23 AM, Joerg Sonnenberger via cfe-commits wrote:

On Thu, Mar 03, 2016 at 04:50:11PM -0800, Nico Weber via cfe-commits wrote:

On Thu, Mar 3, 2016 at 4:28 PM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:


On Thu, Mar 03, 2016 at 02:09:17PM -0800, Nico Weber via cfe-commits wrote:

On Thu, Mar 3, 2016 at 1:50 PM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:


On Thu, Mar 03, 2016 at 07:39:04PM +, Weiming Zhao via cfe-commits
wrote:

Change the option name to -ffile-macro-prefix-to-remove

This still sounds to me like a solution for a very restricted part of a
much more generic problem...


What do you mean?

Storing only the base name of file names is a strict subset of __FILE__
transformations. As mentioned in the linked GCC PR, other uses are
creating build location independent output for larger software projects
etc. For that, reducing to the base name is not an option as it removes
too much information.


But ffile-macro-prefix-to-remove has a general prefix arg, which you can
set to your build dir. This isn't just a "get me the basename" flag (?)

It doesn't help to make sure that path names are always using /usr/src
or src/ and build/ without requiring those to be part of the build
layout. Another instance not handled is symlink trees for controlling
access, where you still want to use the original path etc.

This isn't changing the defaults.  In addition, you could have a mix of 
files in the same binary, some with ffile-macro-prefix-to-remove set, 
and some without.  In cases where the user wants to be explicit about 
full name vs. base name in the source, this patch also provides 
__CLANG_FILE_BASENAME__.


There doesn't seem to be one approach to improve all of the existing use 
cases.  This patch provides two independent tools that can be used to 
improve many of the use cases.


--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Joerg Sonnenberger via cfe-commits]

On Thu, Mar 03, 2016 at 04:50:11PM -0800, Nico Weber via cfe-commits wrote:
> On Thu, Mar 3, 2016 at 4:28 PM, Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> 
> > On Thu, Mar 03, 2016 at 02:09:17PM -0800, Nico Weber via cfe-commits wrote:
> > > On Thu, Mar 3, 2016 at 1:50 PM, Joerg Sonnenberger via cfe-commits <
> > > cfe-commits@lists.llvm.org> wrote:
> > >
> > > > On Thu, Mar 03, 2016 at 07:39:04PM +, Weiming Zhao via cfe-commits
> > > > wrote:
> > > > > Change the option name to -ffile-macro-prefix-to-remove
> > > >
> > > > This still sounds to me like a solution for a very restricted part of a
> > > > much more generic problem...
> > > >
> > >
> > > What do you mean?
> >
> > Storing only the base name of file names is a strict subset of __FILE__
> > transformations. As mentioned in the linked GCC PR, other uses are
> > creating build location independent output for larger software projects
> > etc. For that, reducing to the base name is not an option as it removes
> > too much information.
> >
> 
> But ffile-macro-prefix-to-remove has a general prefix arg, which you can
> set to your build dir. This isn't just a "get me the basename" flag (?)

It doesn't help to make sure that path names are always using /usr/src
or src/ and build/ without requiring those to be part of the build
layout. Another instance not handled is symlink trees for controlling
access, where you still want to use the original path etc.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nico Weber via cfe-commits]

On Thu, Mar 3, 2016 at 4:28 PM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Thu, Mar 03, 2016 at 02:09:17PM -0800, Nico Weber via cfe-commits wrote:
> > On Thu, Mar 3, 2016 at 1:50 PM, Joerg Sonnenberger via cfe-commits <
> > cfe-commits@lists.llvm.org> wrote:
> >
> > > On Thu, Mar 03, 2016 at 07:39:04PM +, Weiming Zhao via cfe-commits
> > > wrote:
> > > > Change the option name to -ffile-macro-prefix-to-remove
> > >
> > > This still sounds to me like a solution for a very restricted part of a
> > > much more generic problem...
> > >
> >
> > What do you mean?
>
> Storing only the base name of file names is a strict subset of __FILE__
> transformations. As mentioned in the linked GCC PR, other uses are
> creating build location independent output for larger software projects
> etc. For that, reducing to the base name is not an option as it removes
> too much information.
>

But ffile-macro-prefix-to-remove has a general prefix arg, which you can
set to your build dir. This isn't just a "get me the basename" flag (?)


>
> Joerg
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Joerg Sonnenberger via cfe-commits]

On Thu, Mar 03, 2016 at 02:09:17PM -0800, Nico Weber via cfe-commits wrote:
> On Thu, Mar 3, 2016 at 1:50 PM, Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> 
> > On Thu, Mar 03, 2016 at 07:39:04PM +, Weiming Zhao via cfe-commits
> > wrote:
> > > Change the option name to -ffile-macro-prefix-to-remove
> >
> > This still sounds to me like a solution for a very restricted part of a
> > much more generic problem...
> >
> 
> What do you mean?

Storing only the base name of file names is a strict subset of __FILE__
transformations. As mentioned in the linked GCC PR, other uses are
creating build location independent output for larger software projects
etc. For that, reducing to the base name is not an option as it removes
too much information.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nico Weber via cfe-commits]

On Thu, Mar 3, 2016 at 1:50 PM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Thu, Mar 03, 2016 at 07:39:04PM +, Weiming Zhao via cfe-commits
> wrote:
> > Change the option name to -ffile-macro-prefix-to-remove
>
> This still sounds to me like a solution for a very restricted part of a
> much more generic problem...
>

What do you mean?


>
> Joerg
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Joerg Sonnenberger via cfe-commits]

On Thu, Mar 03, 2016 at 07:39:04PM +, Weiming Zhao via cfe-commits wrote:
> Change the option name to -ffile-macro-prefix-to-remove

This still sounds to me like a solution for a very restricted part of a
much more generic problem...

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz updated the summary for this revision.
weimingz updated this revision to Diff 49762.
weimingz added a comment.

Change the option name to -ffile-macro-prefix-to-remove


http://reviews.llvm.org/D17741

Files:
  include/clang/Driver/Options.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp
  test/Lexer/file_basename.c

Index: test/Lexer/file_basename.c
===
--- /dev/null
+++ test/Lexer/file_basename.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -E %s | FileCheck %s --check-prefix=ALL --check-prefix=DEFAULT
+
+// RUN: %clang_cc1 -E -ffile-macro-prefix-to-remove=__ALL_DIR__ %s | FileCheck %s --check-prefix=ALL --check-prefix=NO-DIR
+// RUN:%clang -E -ffile-macro-prefix-to-remove=__ALL_DIR__ %s | FileCheck %s --check-prefix=ALL --check-prefix=NO-DIR
+
+// RUN: %clang_cc1 -E -ffile-macro-prefix-to-remove=%s %s | FileCheck %s --check-prefix=ALL --check-prefix=REMOVE-ALL
+// RUN: %clang -E -ffile-macro-prefix-to-remove=%s %s | FileCheck %s --check-prefix=ALL --check-prefix=REMOVE-ALL
+
+// RUN: %clang_cc1 -E -ffile-macro-prefix-to-remove=%s.xyz %s | FileCheck %s --check-prefix=ALL --check-prefix=MISMATCH
+// RUN: %clang -E -ffile-macro-prefix-to-remove= %s | FileCheck %s --check-prefix=ALL --check-prefix=MISMATCH
+
+const char *filename (const char *name) {
+  // ALL:  static const char this_file_basename[] = "file_basename.c";
+  static const char this_file_basename[] = __CLANG_FILE_BASENAME__;
+  // DEFAULT: static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  // NO-DIR:  static const char this_file[] = "file_basename.c";
+  // REMOVE-ALL: static const char this_file[] = "";
+  // MISMATCH: static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  static const char this_file[] = __FILE__;
+  return this_file;
+}
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -12,7 +12,6 @@
 //
 //===--===//
 
-#include "clang/Lex/Preprocessor.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -22,12 +21,15 @@
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -292,6 +294,8 @@
 void Preprocessor::RegisterBuiltinMacros() {
   Ident__LINE__ = RegisterBuiltinMacro(*this, "__LINE__");
   Ident__FILE__ = RegisterBuiltinMacro(*this, "__FILE__");
+  Ident__CLANG_FILE_BASENAME__ =
+  RegisterBuiltinMacro(*this, "__CLANG_FILE_BASENAME__");
   Ident__DATE__ = RegisterBuiltinMacro(*this, "__DATE__");
   Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
   Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
@@ -1509,7 +1513,8 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__ ||
+ II == Ident__CLANG_FILE_BASENAME__) {
 // C99 6.10.8: "__FILE__: The presumed name of the current source file (a
 // character string literal)". This can be affected by #line.
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
@@ -1522,15 +1527,30 @@
 PLoc = SourceMgr.getPresumedLoc(NextLoc);
 if (PLoc.isInvalid())
   break;
-
 NextLoc = PLoc.getIncludeLoc();
   }
 }
 
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
-  FN += PLoc.getFilename();
+  const std::string  =
+  getPreprocessorOpts().__FILE__PrefixToRemove;
+  StringRef Filename(PLoc.getFilename());
+
+  if (II == Ident__CLANG_FILE_BASENAME__)
+FN += llvm::sys::path::filename(Filename);
+  else if (II == Ident__FILE__ && !PrefixToRemove.empty()) {
+if (PrefixToRemove == "__ALL_DIR__") {
+  FN += llvm::sys::path::filename(Filename);
+}
+else if (Filename.find(PrefixToRemove) == 0)
+  FN += Filename.substr(PrefixToRemove.size());
+else
+  FN += Filename;
+  } else
+FN += Filename;
+
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: lib/Frontend/CompilerInvocation.cpp

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Hal Finkel via cfe-commits]

hfinkel added a subscriber: hfinkel.
hfinkel added a comment.

In http://reviews.llvm.org/D17741#367113, @weimingz wrote:

> Add "-f__FILE__-prefix-to-remove" flag to support the trim of the prefix.
>  Passing special value __ALL_DIR__  to remove all dir parts.
>
> For example FILE is /a/b/c
>  -f__FILE__-prefix-to-remove=/a/ will cause FILE be expanded to b/c


I *really* don't like having `__FILE__` in the option name. That will look 
visually disjointing on the command line. How about: 
`-ffile-macro-prefix-to-remove`?


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz updated this revision to Diff 49713.
weimingz added a comment.

Add "-f__FILE__-prefix-to-remove" flag to support the trim of the prefix.
Passing special value __ALL_DIR__  to remove all dir parts.

For example FILE is /a/b/c
-f__FILE__-prefix-to-remove=/a/ will cause FILE be expanded to b/c


http://reviews.llvm.org/D17741

Files:
  include/clang/Driver/Options.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp
  test/Lexer/file_basename.c

Index: test/Lexer/file_basename.c
===
--- /dev/null
+++ test/Lexer/file_basename.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -E %s | FileCheck %s --check-prefix=ALL --check-prefix=DEFAULT
+
+// RUN: %clang_cc1 -E -f__FILE__-prefix-to-remove=__ALL_DIR__ %s | FileCheck %s --check-prefix=ALL --check-prefix=NO-DIR
+// RUN:%clang -E -f__FILE__-prefix-to-remove=__ALL_DIR__ %s | FileCheck %s --check-prefix=ALL --check-prefix=NO-DIR
+
+// RUN: %clang_cc1 -E -f__FILE__-prefix-to-remove=%s %s | FileCheck %s --check-prefix=ALL --check-prefix=REMOVE-ALL
+// RUN: %clang -E -f__FILE__-prefix-to-remove=%s %s | FileCheck %s --check-prefix=ALL --check-prefix=REMOVE-ALL
+
+// RUN: %clang_cc1 -E -f__FILE__-prefix-to-remove=%s.xyz %s | FileCheck %s --check-prefix=ALL --check-prefix=MISMATCH
+// RUN: %clang -E -f__FILE__-prefix-to-remove= %s | FileCheck %s --check-prefix=ALL --check-prefix=MISMATCH
+
+const char *filename (const char *name) {
+  // ALL:  static const char this_file_basename[] = "file_basename.c";
+  static const char this_file_basename[] = __CLANG_FILE_BASENAME__;
+  // DEFAULT: static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  // NO-DIR:  static const char this_file[] = "file_basename.c";
+  // REMOVE-ALL: static const char this_file[] = "";
+  // MISMATCH: static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  static const char this_file[] = __FILE__;
+  return this_file;
+}
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -12,7 +12,6 @@
 //
 //===--===//
 
-#include "clang/Lex/Preprocessor.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -22,12 +21,15 @@
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -292,6 +294,8 @@
 void Preprocessor::RegisterBuiltinMacros() {
   Ident__LINE__ = RegisterBuiltinMacro(*this, "__LINE__");
   Ident__FILE__ = RegisterBuiltinMacro(*this, "__FILE__");
+  Ident__CLANG_FILE_BASENAME__ =
+  RegisterBuiltinMacro(*this, "__CLANG_FILE_BASENAME__");
   Ident__DATE__ = RegisterBuiltinMacro(*this, "__DATE__");
   Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
   Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
@@ -1509,7 +1513,8 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__ ||
+ II == Ident__CLANG_FILE_BASENAME__) {
 // C99 6.10.8: "__FILE__: The presumed name of the current source file (a
 // character string literal)". This can be affected by #line.
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
@@ -1522,15 +1527,28 @@
 PLoc = SourceMgr.getPresumedLoc(NextLoc);
 if (PLoc.isInvalid())
   break;
-
 NextLoc = PLoc.getIncludeLoc();
   }
 }
 
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
-  FN += PLoc.getFilename();
+  const std::string  =
+  getPreprocessorOpts().__FILE__PrefixToRemove;
+  StringRef Filename(PLoc.getFilename());
+  if (II == Ident__CLANG_FILE_BASENAME__)
+FN += llvm::sys::path::filename(Filename);
+  else if (II == Ident__FILE__ && !PrefixToRemove.empty()) {
+if (PrefixToRemove == "__ALL_DIR__")
+  FN += llvm::sys::path::filename(Filename);
+else if (Filename.find(PrefixToRemove) == 0)
+  FN += Filename.substr(PrefixToRemove.size());
+else
+  FN += Filename;
+  } else
+FN += Filename;
+
   Lexer::Stringify(FN);
  

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Ben Craig via cfe-commits]

bcraig added a comment.

In http://reviews.llvm.org/D17741#365457, @thakis wrote:

> > I think we can do this separately. A "basename" macro is easier for 
> > programmers to use and no build system change needed.
>
>
> Hm, I would think that adding a flag to your CFLAGS is easier than getting 
> all your dependencies to use a clang-only new macro…


Depends a lot on the person and code base.
**Pro-macro situations:**
I've had code bases where the bulk of the code refused to use compiler / 
environment provided preprocessor tokens.  In those environments, all of the 
uses of __FILE__ and the like were in one place.  Adding a compiler test and 
switching to __CLANG_FILE_BASENAME__ would be easy in those code bases.  
Changing CFLAGS in dozens of different compiles wouldn't.

I've also seen uses of __FILE__ that would break if the file name changed 
drastically and unexpectedly.

**Pro-compilation switch situation:**
If __FILE__ is used in a lot of places, and the files are compiled with 
multiple compilers, then switching to a different macro would be problematic.  
If it is easy to change CFLAGS in all of your builds then the compilation 
switch makes sense.


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Sean Silva via cfe-commits]

On Tue, Mar 1, 2016 at 10:54 AM, Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> thakis added a comment.
>
> > I think we can do this separately. A "basename" macro is easier for
> programmers to use and no build system change needed.
>
>
> Hm, I would think that adding a flag to your CFLAGS is easier than getting
> all your dependencies to use a clang-only new macro…
>

Yeah, we have gotten an internal request for similar functionality and it
definitely was a "whole build" granularity (i.e. CFLAGS) that the behavior
was desired. That being said, __FILE_BASENAME__ might be useful to have in
its own right.

-- Sean Silva


>
>
> http://reviews.llvm.org/D17741
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nico Weber via cfe-commits]

thakis added a comment.

> I think we can do this separately. A "basename" macro is easier for 
> programmers to use and no build system change needed.


Hm, I would think that adding a flag to your CFLAGS is easier than getting all 
your dependencies to use a clang-only new macro…


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Ben Craig via cfe-commits]

bcraig added a comment.

LGTM.  You should probably wait for someone else to approve it though.


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz added a comment.

In http://reviews.llvm.org/D17741#364954, @thakis wrote:

> In http://reviews.llvm.org/D17741#364931, @weimingz wrote:
>
> > In http://reviews.llvm.org/D17741#364756, @thakis wrote:
> >
> > > Instead of doing this, would it make sense to have a flag like 
> > > -ffile-basename that changes what __FILE__ expands to?
> > >
> > > I had wished I'd be able to have some control over __FILE__ (I'd like to 
> > > say "make all __FILE__s relative to this given directory for my use 
> > > case), and changing __FILE__ to something else in all the world's code 
> > > isn't easy – so maybe having a flag that provides some control over 
> > > __FILE__ instead of adding a separate macro would be a good idea.
> >
> >
> > do you mean this?
> >  if source file is /a/b/c/d/foo.c and if  -ffile-name-stem-remove=/a/b/c, 
> > then _FILE_ will be expanded to "d/foo.c" ?
>
>
> Yes, something like that. Maybe it could look like 
> `-f__file__-expansion=basename` (to make __FILE__ expand to just the 
> basename, what you want), `-f__file__-expansion=relative-to:/a/b/c` to make 
> it relative to a given path.


I think we can do this separately. A "basename" macro is easier for programmers 
to use and no build system change needed.


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nico Weber via cfe-commits]

thakis added a comment.

In http://reviews.llvm.org/D17741#364931, @weimingz wrote:

> In http://reviews.llvm.org/D17741#364756, @thakis wrote:
>
> > Instead of doing this, would it make sense to have a flag like 
> > -ffile-basename that changes what __FILE__ expands to?
> >
> > I had wished I'd be able to have some control over __FILE__ (I'd like to 
> > say "make all __FILE__s relative to this given directory for my use case), 
> > and changing __FILE__ to something else in all the world's code isn't easy 
> > – so maybe having a flag that provides some control over __FILE__ instead 
> > of adding a separate macro would be a good idea.
>
>
> do you mean this?
>  if source file is /a/b/c/d/foo.c and if  -ffile-name-stem-remove=/a/b/c, 
> then _FILE_ will be expanded to "d/foo.c" ?


Yes, something like that. Maybe it could look like 
`-f__file__-expansion=basename` (to make __FILE__ expand to just the basename, 
what you want), `-f__file__-expansion=relative-to:/a/b/c` to make it relative 
to a given path.


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz updated this revision to Diff 49445.
weimingz added a comment.

rename the macro to CLANG_FILE_BASENAME per Ben's comments.


http://reviews.llvm.org/D17741

Files:
  include/clang/Lex/Preprocessor.h
  lib/Lex/PPMacroExpansion.cpp
  test/Lexer/file_basename.c

Index: test/Lexer/file_basename.c
===
--- /dev/null
+++ test/Lexer/file_basename.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -E %s | FileCheck %s
+
+const char *filename (const char *name) {
+  // CHECK:  static const char this_file_basename[] = "file_basename.c";
+  static const char this_file_basename[] = __CLANG_FILE_BASENAME__;
+  // CHECK:  static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  static const char this_file[] = __FILE__;
+  return this_file;
+}
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -292,6 +293,8 @@
 void Preprocessor::RegisterBuiltinMacros() {
   Ident__LINE__ = RegisterBuiltinMacro(*this, "__LINE__");
   Ident__FILE__ = RegisterBuiltinMacro(*this, "__FILE__");
+  Ident__CLANG_FILE_BASENAME__ =
+  RegisterBuiltinMacro(*this, "__CLANG_FILE_BASENAME__");
   Ident__DATE__ = RegisterBuiltinMacro(*this, "__DATE__");
   Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
   Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
@@ -1509,7 +1512,8 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__ ||
+ II == Ident__CLANG_FILE_BASENAME__) {
 // C99 6.10.8: "__FILE__: The presumed name of the current source file (a
 // character string literal)". This can be affected by #line.
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
@@ -1530,7 +1534,9 @@
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
-  FN += PLoc.getFilename();
+  FN += II == Ident__CLANG_FILE_BASENAME__
+? llvm::sys::path::filename(PLoc.getFilename())
+: PLoc.getFilename();
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: include/clang/Lex/Preprocessor.h
===
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -119,6 +119,7 @@
 
   /// Identifiers for builtin macros and other builtins.
   IdentifierInfo *Ident__LINE__, *Ident__FILE__;   // __LINE__, __FILE__
+  IdentifierInfo *Ident__CLANG_FILE_BASENAME__;//  __FILE_BASENAME__
   IdentifierInfo *Ident__DATE__, *Ident__TIME__;   // __DATE__, __TIME__
   IdentifierInfo *Ident__INCLUDE_LEVEL__;  // __INCLUDE_LEVEL__
   IdentifierInfo *Ident__BASE_FILE__;  // __BASE_FILE__


Index: test/Lexer/file_basename.c
===
--- /dev/null
+++ test/Lexer/file_basename.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -E %s | FileCheck %s
+
+const char *filename (const char *name) {
+  // CHECK:  static const char this_file_basename[] = "file_basename.c";
+  static const char this_file_basename[] = __CLANG_FILE_BASENAME__;
+  // CHECK:  static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  static const char this_file[] = __FILE__;
+  return this_file;
+}
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -292,6 +293,8 @@
 void Preprocessor::RegisterBuiltinMacros() {
   Ident__LINE__ = RegisterBuiltinMacro(*this, "__LINE__");
   Ident__FILE__ = RegisterBuiltinMacro(*this, "__FILE__");
+  Ident__CLANG_FILE_BASENAME__ =
+  RegisterBuiltinMacro(*this, "__CLANG_FILE_BASENAME__");
   Ident__DATE__ = RegisterBuiltinMacro(*this, "__DATE__");
   Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
   Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
@@ -1509,7 +1512,8 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__ ||
+ II == Ident__CLANG_FILE_BASENAME__) {
   

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz added a comment.

In http://reviews.llvm.org/D17741#364756, @thakis wrote:

> Instead of doing this, would it make sense to have a flag like 
> -ffile-basename that changes what __FILE__ expands to?
>
> I had wished I'd be able to have some control over __FILE__ (I'd like to say 
> "make all __FILE__s relative to this given directory for my use case), and 
> changing __FILE__ to something else in all the world's code isn't easy – so 
> maybe having a flag that provides some control over __FILE__ instead of 
> adding a separate macro would be a good idea.


do you mean this?
if source file is /a/b/c/d/foo.c and if  -ffile-name-stem-remove=/a/b/c, then 
_FILE_ will be expanded to "d/foo.c" ?


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Joerg Sonnenberger via cfe-commits]

joerg added a subscriber: joerg.
joerg added a comment.

I've added functionality like that to NetBSD's GCC 
 a long time ago. That 
functionality is useful for a variety of situations, cutting down file space or 
canonicalization are two uses.


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nico Weber via cfe-commits]

thakis added a subscriber: thakis.
thakis added a comment.

Instead of doing this, would it make sense to have a flag like -ffile-basename 
that changes what __FILE__ expands to?

I had wished I'd be able to have some control over __FILE__ (I'd like to say 
"make all __FILE__s relative to this given directory for my use case), and 
changing __FILE__ to something else in all the world's code isn't easy – so 
maybe having a flag that provides some control over __FILE__ instead of adding 
a separate macro would be a good idea.


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Ben Craig via cfe-commits]

bcraig added a subscriber: bcraig.
bcraig added a comment.

Note: this doesn't count as an official "LGTM".

The code change seems fine to me.  I think this has been implemented in gcc as 
well, but I don't recall for certain.  If this has been implemented in gcc, 
then I would expect the semantics to be the same.  If it hasn't been 
implemented in gcc, then we might want to pick a different name for the macro 
(e.g. __CLANG_FILE_BASENAME__).


http://reviews.llvm.org/D17741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Weiming Zhao via cfe-commits]

weimingz created this revision.
weimingz added a subscriber: cfe-commits.

__FILE__ will be expanded to the full path. In some embedded system scenarios, 
the final images is linked from many objs and the image size is a very 
important factor.
The full filenames can occupy a lot space in the string pool and most of the 
time,  knowing the base name is sufficient.
__FILE_BASENAME__ can be used in this scenario.

http://reviews.llvm.org/D17741

Files:
  include/clang/Lex/Preprocessor.h
  lib/Lex/PPMacroExpansion.cpp
  test/Lexer/file_basename.c

Index: test/Lexer/file_basename.c
===
--- /dev/null
+++ test/Lexer/file_basename.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -E %s | FileCheck %s
+
+const char *filename (const char *name) {
+  // CHECK:  static const char this_file_basename[] = "file_basename.c";
+  static const char this_file_basename[] = __FILE_BASENAME__;
+  // CHECK:  static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  static const char this_file[] = __FILE__;
+  return this_file;
+}
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -292,6 +293,7 @@
 void Preprocessor::RegisterBuiltinMacros() {
   Ident__LINE__ = RegisterBuiltinMacro(*this, "__LINE__");
   Ident__FILE__ = RegisterBuiltinMacro(*this, "__FILE__");
+  Ident__FILE_BASENAME__ = RegisterBuiltinMacro(*this, "__FILE_BASENAME__");
   Ident__DATE__ = RegisterBuiltinMacro(*this, "__DATE__");
   Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
   Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
@@ -1509,7 +1511,8 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__ ||
+ II == Ident__FILE_BASENAME__) {
 // C99 6.10.8: "__FILE__: The presumed name of the current source file (a
 // character string literal)". This can be affected by #line.
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
@@ -1530,7 +1533,9 @@
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
-  FN += PLoc.getFilename();
+  FN += II == Ident__FILE_BASENAME__
+  ? llvm::sys::path::filename(PLoc.getFilename())
+  : PLoc.getFilename();
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: include/clang/Lex/Preprocessor.h
===
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -119,6 +119,7 @@
 
   /// Identifiers for builtin macros and other builtins.
   IdentifierInfo *Ident__LINE__, *Ident__FILE__;   // __LINE__, __FILE__
+  IdentifierInfo *Ident__FILE_BASENAME__;  //  __FILE_BASENAME__
   IdentifierInfo *Ident__DATE__, *Ident__TIME__;   // __DATE__, __TIME__
   IdentifierInfo *Ident__INCLUDE_LEVEL__;  // __INCLUDE_LEVEL__
   IdentifierInfo *Ident__BASE_FILE__;  // __BASE_FILE__


Index: test/Lexer/file_basename.c
===
--- /dev/null
+++ test/Lexer/file_basename.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -E %s | FileCheck %s
+
+const char *filename (const char *name) {
+  // CHECK:  static const char this_file_basename[] = "file_basename.c";
+  static const char this_file_basename[] = __FILE_BASENAME__;
+  // CHECK:  static const char this_file[] = "{{.*}}/Lexer/file_basename.c";
+  static const char this_file[] = __FILE__;
+  return this_file;
+}
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -292,6 +293,7 @@
 void Preprocessor::RegisterBuiltinMacros() {
   Ident__LINE__ = RegisterBuiltinMacro(*this, "__LINE__");
   Ident__FILE__ = RegisterBuiltinMacro(*this, "__FILE__");
+  Ident__FILE_BASENAME__ = RegisterBuiltinMacro(*this, "__FILE_BASENAME__");
   Ident__DATE__ = RegisterBuiltinMacro(*this, "__DATE__");
   Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
   Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
@@ -1509,7 +1511,8 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);