[PATCH] D144878: __builtin_FILE_NAME()

Ilya Karapsin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 9 23:51:12 PST 2023


karapsinie marked an inline comment as done.
karapsinie added a comment.

In D144878#4172639 <https://reviews.llvm.org/D144878#4172639>, @aaron.ballman wrote:

> In D144878#4171234 <https://reviews.llvm.org/D144878#4171234>, @karapsinie wrote:
>
>> PTAL.
>
> Have you seen the comments on the GCC issue that @MaskRay filed? Is that something we should do as well? (It doesn't have to be part of this patch, but it'd be good to ensure we're collaborating with GCC so we get the same builtin functionality in this space.)
>
> There should be a release note and documentation for the new functionality. One thing to consider for the docs is explaining what's going on with text encodings (and perhaps that text applies to this entire class of file-related builtins). e.g., if the system code page is Shift-JIS does this builtin return the text in Shift-JIS or UTF-8 or something else?

This is my first open-source commit.
I don't know where or what to write.
Maybe you will do it or tell me how to do it?



================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:1553
         int a = d;
-        union { 
+        union {
           int b;
----------------
aaron.ballman wrote:
> You should back out the spurious whitespace changes (I'd recommend setting your editor to not discard trailing whitespace on save as that's a common culprit for this).
Ok.


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

https://reviews.llvm.org/D144878



More information about the llvm-commits mailing list