[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc511963d5adb: [clang] Fix length threshold for MicrosoftMangle md5 hash (authored by mibintc). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

mibintc updated this revision to Diff 303640. mibintc added a comment. I submitted the test improvements separately, i'm going to push this version, thanks all Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90714/new/

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

dblaikie added a comment. In D90714#2379785 , @mibintc wrote: > you're right about the regex syntax. i think the 4088 is over the max > repetition count and there's also a problem with balanced. not sure what i > did wrong here. if you have a chance to

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

mibintc added a comment. you're right about the regex syntax. i think the 4088 is over the max repetition count and there's also a problem with balanced. not sure what i did wrong here. if you have a chance to look at this otherwise i'll toil over the weekend, i'm off for the day--cheers.

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

dblaikie accepted this revision. dblaikie added a comment. Generally looks good, thanks! The inline comment trying to understand the `4095 - 4088 == 7` math I think should be answered (possibly in the form of the test/CHECK rephrasing I mentioned to clarify what the extra 7 characters are, and

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

mibintc updated this revision to Diff 303477. mibintc added a comment. Fixed test case according to @dblaikie 's suggestion and input, thanks a lot David, the test case is certainly a lot easier to read and understand now. Anything else? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

dblaikie added a comment. In D90714#2377231 , @mibintc wrote: > I'm sorry, I don't see how to build up an identifier with an arbitrary number > of characters using the token pasting, for example an ident with 4095 > characters is not a power of 2. I

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

mibintc added a comment. I'm sorry, I don't see how to build up an identifier with an arbitrary number of characters using the token pasting, for example an ident with 4095 characters is not a power of 2. I tried pasting together X2048 X1024 X512 etc but that doesn't work fu.cpp:12:18: error:

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

dblaikie added a comment. In D90714#2376667 , @rnk wrote: > lgtm > > I think the test case looks good as is, FWIW. Token pasting might make it > more readable, but it's improving the existing test, and not necessarily in > scope for this patch. Oh,

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

dblaikie added a comment. In D90714#2375911 , @mibintc wrote: > In D90714#2374913 , @dblaikie wrote: > >> Since the same code is used to mangle all these things, probably just test >> one of them? >> >> Could use

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm I think the test case looks good as is, FWIW. Token pasting might make it more readable, but it's improving the existing test, and not necessarily in scope for this patch. Repository: rG

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

mibintc added a comment. In D90714#2374913 , @dblaikie wrote: > Since the same code is used to mangle all these things, probably just test > one of them? > > Could use macros to stamp out longer names without having to write them out > manually? Not

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

dblaikie added a comment. Since the same code is used to mangle all these things, probably just test one of them? Could use macros to stamp out longer names without having to write them out manually? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

mibintc added a comment. In D90714#2372458 , @dblaikie wrote: > Test case? Thanks, done Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90714/new/ https://reviews.llvm.org/D90714

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

mibintc updated this revision to Diff 302902. mibintc added a comment. Add lit test case, based on test case in bug report Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90714/new/ https://reviews.llvm.org/D90714 Files:

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

dblaikie added a comment. Test case? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90714/new/ https://reviews.llvm.org/D90714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D90714: [clang]Fix length threshold for MicrosoftMangle md5 hash

mibintc created this revision. mibintc added a reviewer: majnemer. Herald added a project: clang. mibintc requested review of this revision. Fix off-by-one bug in determining the threshold for when want to shorten very large external names, Microsoft compatibility. This fixes the bug reported