[flang] [libc] [libunwind] [libcxx] [clang-tools-extra] [libcxxabi] [lld] [llvm] [lldb] [clang] [compiler-rt] [mlir] PR#72453 : Exceeding maximum file name length (PR #72654)

2023-11-17 Thread Todd A. Anderson via cfe-commits

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)

2023-11-17 Thread Todd A. Anderson via cfe-commits


@@ -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)

2023-11-17 Thread Todd A. Anderson via cfe-commits

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)

2023-11-17 Thread Todd A. Anderson via cfe-commits


@@ -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)

2023-11-17 Thread Todd A. Anderson via cfe-commits


@@ -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)

2023-11-17 Thread Todd A. Anderson via cfe-commits


@@ -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)

2023-11-17 Thread Todd A. Anderson via cfe-commits


@@ -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)

2023-11-18 Thread Todd A. Anderson via cfe-commits

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)

2023-11-18 Thread Todd A. Anderson via cfe-commits

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)

2023-11-18 Thread Todd A. Anderson via cfe-commits


@@ -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)

2023-11-18 Thread Todd A. Anderson via cfe-commits


@@ -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)

2023-11-20 Thread Todd A. Anderson via cfe-commits


@@ -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