[flang] [libc] [libunwind] [libcxx] [clang-tools-extra] [libcxxabi] [lld] [llvm] [lldb] [clang] [compiler-rt] [mlir] PR#72453 : Exceeding maximum file name length (PR #72654)
https://github.com/DrTodd13 requested changes to this pull request. I don't think these changes as they are solve the problem. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libunwind] [lldb] [lld] [flang] [compiler-rt] [clang-tools-extra] [llvm] [libc] [libcxx] [mlir] [libcxxabi] PR#72453 : Exceeding maximum file name length (PR #72654)
@@ -94,7 +96,7 @@ void printGraphForFunction(Function &F, GraphT Graph, StringRef Name, raw_fd_ostream File(Filename, EC, sys::fs::OF_TextWithCRLF); DrTodd13 wrote: If Filename is too long, then EC here will already contain an error code. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libcxxabi] [clang] [mlir] [compiler-rt] [clang-tools-extra] [libcxx] [flang] [lldb] [llvm] [libc] [lld] PR#72453 : Exceeding maximum file name length (PR #72654)
https://github.com/DrTodd13 edited https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[mlir] [llvm] [libunwind] [libcxx] [libcxxabi] [libc] [clang] [lldb] [compiler-rt] [lld] [clang-tools-extra] [flang] PR#72453 : Exceeding maximum file name length (PR #72654)
@@ -310,7 +312,7 @@ void WriteDOTGraphToFile(Function &F, GraphT &&Graph, std::string GraphName = DOTGraphTraits::getGraphName(Graph); std::string Title = GraphName + " for '" + F.getName().str() + "' function"; - if (!EC) + if (!EC && (Filename.length() <= MAX_FILENAME_LEN)) DrTodd13 wrote: Same issue as above. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [libcxx] [flang] [clang] [libc] [mlir] [lldb] [compiler-rt] [libunwind] [lld] [libcxxabi] PR#72453 : Exceeding maximum file name length (PR #72654)
@@ -94,7 +96,7 @@ void printGraphForFunction(Function &F, GraphT Graph, StringRef Name, raw_fd_ostream File(Filename, EC, sys::fs::OF_TextWithCRLF); std::string GraphName = DOTGraphTraits::getGraphName(Graph); - if (!EC) + if (!EC && (Filename.length() <= MAX_FILENAME_LEN)) DrTodd13 wrote: Not sure this does anything since if filename is too long then an error code EC will already be set. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libc] [libcxxabi] [mlir] [llvm] [compiler-rt] [lldb] [flang] [lld] [libcxx] [clang-tools-extra] [libunwind] PR#72453 : Exceeding maximum file name length (PR #72654)
@@ -94,7 +96,7 @@ void printGraphForFunction(Function &F, GraphT Graph, StringRef Name, raw_fd_ostream File(Filename, EC, sys::fs::OF_TextWithCRLF); DrTodd13 wrote: I needed to work around this bug when I found it so in my local version I did a (Name.str() + "." + F.getName().str()).substr(0,250) + ".dot". In this way, I just truncate the lengthy filename to 250 characters before adding the ".dot". However, this approach has the problem that two functions who names differ only past the 250 character mark will try to write to the same file. I don't know what the right solution is here and that's why I posted it for discussion. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [mlir] [libc] [libcxxabi] [clang-tools-extra] [libunwind] [libcxx] [clang] [lld] [lldb] [flang] [llvm] PR#72453 : Exceeding maximum file name length (PR #72654)
@@ -280,7 +282,7 @@ class DOTGraphTraitsModulePrinterWrapperPass : public ModulePass { raw_fd_ostream File(Filename, EC, sys::fs::OF_TextWithCRLF); std::string Title = DOTGraphTraits::getGraphName(Graph); -if (!EC) +if (!EC && (Filename.length() <= MAX_FILENAME_LEN)) DrTodd13 wrote: Same issue as above. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lld] [llvm] [clang-tools-extra] [lldb] [compiler-rt] [libunwind] [libc] [mlir] [clang] [libcxxabi] [libcxx] [flang] PR#72453 : Exceeding maximum file name length (PR #72654)
https://github.com/DrTodd13 edited https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [libc] [libunwind] [clang-tools-extra] [flang] [libcxxabi] [lldb] [mlir] [libcxx] [compiler-rt] [lld] [clang] PR#72453 : Exceeding maximum file name length (PR #72654)
https://github.com/DrTodd13 requested changes to this pull request. I like the idea of that new function but I don't think it does what you intended. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[mlir] [lldb] [libc] [libcxxabi] [flang] [compiler-rt] [lld] [clang-tools-extra] [libcxx] [clang] [llvm] [libunwind] PR#72453 : Exceeding maximum file name length (PR #72654)
@@ -83,10 +85,29 @@ struct DOTGraphTraitsViewer StringRef Name; }; +static void shortenFileName(std::string &FN, unsigned char len = 250) { + + FN = FN.substr(0, len); + if (nameObj.empty()) +nameObj.push_back(FN); + + else { +for (auto it = nameObj.begin(); it != nameObj.end(); it++) { + if (*it == FN) { +FN = FN.substr(0, --len); DrTodd13 wrote: Let's say that I have 3 filenames submitted in a row that share the same first 250 characters. Line 92 will add the first one truncated to 250. Then, line 97 will add the second one truncated to 249. But then, for the third one, the first time through the nameObj list, it is going to match with the first name you added, decrement len by 1 and then add to nameObj. So, I think the 2nd and 3rd names in your list are going to be duplicates. Maybe a more classic way of doing this would be to use a set instead of a list, check if the FN truncated to 250 is in the set and if not add it. If it is then truncate to 249 and repeat the process of checking if it is in the set. Add if it isn't and if it is then truncate to 248 and keep repeating the process until it can be added. Of course, this approach has the problem that if you have more than 250 names that shared the first 250 characters then len would go to 0 and you'd try to have a filename that was empty. Then the 251st time you tried it you'd have a negative substr len. If that is equivalent to 0 len then you could end up in an infinite loop. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [compiler-rt] [libcxx] [mlir] [lldb] [lld] [flang] [clang-tools-extra] [libc] [libcxxabi] [llvm] [clang] PR#72453 : Exceeding maximum file name length (PR #72654)
@@ -17,6 +17,8 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/GraphWriter.h" +static std::vector nameObj; DrTodd13 wrote: I don't know if there is some LLVM code approach that would dictate what to do here. We can have some kind of a data structure to remember names to try to avoid duplicates or we can accept that there may be duplicates but just make them probabilistically rare. I guess I am leaning toward the latter where you do something like take up to a certain number of characters of the filename as they are but then you take the rest of the characters of the filename and hash them and add the hash to the filename. This latter approach works even if you are going in and out of llvm whereas having a data structure to remember names would only work in the context of single execution of LLVM. Again, that is just my opinion and I'm not sure what people with approval authority here will say. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libc] [llvm] [clang] [libunwind] [lld] [libcxx] [lldb] [compiler-rt] [libcxxabi] [mlir] [flang] PR#72453 : Exceeding maximum file name length (PR #72654)
@@ -83,10 +85,29 @@ struct DOTGraphTraitsViewer StringRef Name; }; +static void shortenFileName(std::string &FN, unsigned char len = 250) { + + FN = FN.substr(0, len); + if (nameObj.empty()) +nameObj.push_back(FN); + + else { +for (auto it = nameObj.begin(); it != nameObj.end(); it++) { + if (*it == FN) { +FN = FN.substr(0, --len); DrTodd13 wrote: @shahidiqbal13 Thanks for the changes. Just one minor point, you don't need the initial "if" to check for empty set anymore. The first time through the loop if it isn't found in the set then it will add it. Also, I don't know how much we need to consider this but sometimes ordering will be reversed and so the filename assigned to a function would be different in your approach based on which function is encountered first. With the hash approach this wouldn't happen. At this point, we need input from regular LLVM devs in terms of how they like to do things. https://github.com/llvm/llvm-project/pull/72654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits