Match Clang's sorting, so that longer (more specific) prefix paths
will match before less specific paths.
Details
- Reviewers
MaskRay dblaikie raj.khem - Group Reviewers
debug-info - Commits
- rG3922ec46b84a: [MCContext] Reverse order of DebugPrefixMap sort for generated assembly debug…
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I'm thinking about changing all the definitions of DebugPrefixMap like structures to just one named type, so that this is consistent everywhere.
The test doesn't help. %t=... does not match the path component %t.foo. A nested directory is needed to test the situation.
You probably want to swap the order of the two -fdebug-prefix-map= for -dwarf-version=5 to get some variance.
Sure it does. It's somewhat undocumented, and arguably a bug in its own right (I was torn when I implemented the original prefix mapping stuff), but it's a simple prefix substitution. /t=/lu turns /tmp into /lump happily.
In this case without the header change, that unit test turns the file path into /broken_root.foo/src.s.
You probably want to swap the order of the two -fdebug-prefix-map= for -dwarf-version=5 to get some variance.
I like this suggestion. The order of the parameters, so let's ensure that both work.
Hmm. While this is true, it depends on how replace_path_prefix is called, and that seems inconsistent. That *is* a bug. Clang gives me multiple file names when I try that there. In llvm-mc, though it's always the string prefix.
I'll use subdirectories, since I now believe it *should* fail in the /t=/lu changing /tmp to /lump.
llvm/test/MC/ELF/debug-prefix-map.s | ||
---|---|---|
1–2 | mkdir -p %t.foo/bar |
llvm/test/MC/ELF/debug-prefix-map.s | ||
---|---|---|
1–2 | I thought about that, but I also think doing it separately makes it more clear to readers that both paths will be used. |
llvm/test/MC/ELF/debug-prefix-map.s | ||
---|---|---|
1–2 | You may keep em, then it should still be clear |
One more attemt to fix Windows tests before I actually install a
Windows VM to work on it..
Since I don't have commit access, can one of you commit this for me, please?
Assuming you're happy with the test updates.
Is it possible to get this into 15? Or do we need to wait for 16 at this point?
llvm/include/llvm/MC/MCContext.h | ||
---|---|---|
193 | What happened to the const qualifier here? |
llvm/include/llvm/MC/MCContext.h | ||
---|---|---|
193 | const key is very uncommon and constness usually doesn't matter (value_type adds const to the key type anyway). |
I've created https://github.com/llvm/llvm-project/issues/57437 for 15.0.0 backport. If it is too late, we can request 15.0.1 backport.
Thanks!
llvm/include/llvm/MC/MCContext.h | ||
---|---|---|
193 | Ah, missed that in the std::map spec. It makes sense that keys are always const, just didn't know it was guaranteed. Thanks. |
What happened to the const qualifier here?