- User Since
- May 9 2013, 11:10 AM (288 w, 6 d)
This is small enough that I'd prefer to see it incorporated into the patch with the "real" warning, rather than have this temporary placeholder diagnostic.
Tue, Nov 20
Mon, Nov 19
Fri, Nov 16
Regarding the bit about environment variables, probably the right thing to do is add a lit.local.cfg to the FileCheck test directory, that zaps the environment variables. Then most individual FileCheck tests can assume a default environment, and tests for specific non-defaults can set the environment variables directly in the test file.
It would mean you can't run the FileCheck tests *from lit* with an environment variable set. Unless we invented another env var that lit.local.cfg would look for... but that's for another patch, at this point.
It really seems like DiagList and AnnotationList ought to be vectors, not lists. They are append-only, and AnnotationList gets converted to a vector anyway to sort it. The code doesn't depend on the element-pointer stability guarantee of a list, except in one place noted below which can be fixed.
It's quite possible a vector would perform less well, in the face of many diags/annotations, but as the diags/annotations are the unusual case, performance is not really a big consideration.
Delete unneeded ctor. Clang-format.
Thu, Nov 15
Remove new .def file, move content into DebugInfoFlags.def.
Fix one oversight in Clang part, lit tests now all pass.
Wed, Nov 14
My experiments with clang-format show that it is aware of this convention. It will change /*Prefix*/nullptr to /*Prefix*/ nullptr (inserting a space) and will also change /*Prefix=*/ nullptr to /*Prefix=*/nullptr (removing the space, if the comment ends with =).
Tue, Nov 13
Okay, clearly I have not been paying attention. It's not that lit -v enables the warning, it is simply not suppressing the message. This is why lit without -v appears to turn it "off." I get it now. As far as FileCheck itself is concerned, this is always-on.
Have you tried running the entire suite with lit -v to see how often the diagnostic comes up? Now that you don't emit a diagnostic for the multiple-prefix case.
I don't really see a need for a new directive, it's convenient but does nothing you couldn't do with existing directives.
It does have some downside, in that it's supported only by the LLVM integrated assembler.
Mon, Nov 12
Fri, Nov 9
Make DwarfDebug responsible for picking which section to use.
I hadn't thought about -v; I don't think the new diag is currently under -v? The first test isn't using that option...
Thu, Nov 8
Because there is nothing to indicate that a line contains a FileCheck directive, other than that it pattern-matches a user-specified string with an optional suffix, we can't reliably report all typos. So for example:
CHECK-LABEL: A CHECK: B CHEKC: C
the third line can't be diagnosed by FileCheck under any reasonable heuristic. So, anything we do is a best-effort, and ideally produces no false positives by default.
Wed, Nov 7
Seems like it would easy enough to base pass/fail on getting a perfect score, for the purpose of debuginfo-tests.
Just to throw the idea out there, why not abandon debuginfo-tests entirely and try using Dexter for this. Dexter's design center is debug-info quality, not correctness, but it already knows how to drive several different debuggers on multiple platforms.
Dexter would have to become an LLVM project tool, and I'm not sure how Sony management feels about that, but I think this would be an awesome use-case.
I think what we've determined so far is that some uses of CHECK-LABEL simply misunderstand the LABEL suffix, which IMO is because the suffix is very poorly named. Renaming it to something harder to misunderstand would be a lot of churn but ultimately benefit the project's tests. The process of updating all tests with LABEL would necessarily find all the dodgy cases. And with a better-understood FileCheck feature, ultimately I think there would be fewer cases of bad tests.
(It would also help if people responsibly made sure their tests failed when their patch was NOT applied, but we have no process for this.)
Tue, Nov 6
A patch to report an error on multiple -D of the same variable is probably a good thing, particularly since it doesn't work properly.
We already do this for X86, would it be better to make the check in non-target-specific code? (Just asking, not expressing an opinion.)
Even when DWARF adds something in response to the needs of a specific language, it doesn't use language names in the tags or attributes. So, I think putting "Fortran" in the symbol names here would be inconsistent with that. It's fine to describe it as a Fortran attribute in commentary.
As a hypothetical example, C++ has the concept of a "pure" virtual method declaration, and somebody might come up with a reason to mark those with the new Pure flag.
Which is another reason not to start overloading the existing flags based on assumptions about which languages use which flags. I've started sketching out what it would take to add a separate DISPFlags type/field in DISubprogram.
Ping. For my next trick I was going to do the patch that emits type units in .debug_info instead of .debug_types, for DWARF v5. Would it help to have that up first?
Fri, Nov 2
Procedural point: When you upload a diff to Phabricator, please include more context (-U999999 is typical).
Thu, Nov 1
Please review all comments to ensure proper punctuation. There are a lot of missing full-stops.
Presumably this addresses the bad debugging experience for the sample from the PR; does it have any other good/bad effect on the Dexter corpus?
Wed, Oct 31
Tue, Oct 30
I reordered the units in the test file, because llvm-dwarfdump will dump all the .debug_info bits before going on to the .debug_types bits. The order of the sections in the file doesn't matter, but the order of the CHECKs does, and this was the simplest way to deal with it.
Does this lose debug information? I do very little with optimization passes so I don't know, but it would be nice to keep it in mind.
Fri, Oct 26
Wed, Oct 24
Tue, Oct 23
I've put a note out to the DWARF committee to see if we can streamline the assignment of language codes. As long as there is a source of truth for these, we shouldn't have to wait for DWARF 6 to be published.
The idea has come up before, and now with a motivating case I think it's likely to happen. So if you could hold off a little longer (probably a few days) I can have an answer for you on that point.
Oct 22 2018
Oct 19 2018
Following up from the BoF and the corresponding email thread:
I think we want to distinguish the general guideline/intent (support N years/releases) from the firm statement "we are actually going to require *this* minimum version starting on *this* date." There seems to be some resistance to having the latter happen on a strict timeline.
Having separated out those two things, we should specify a minimum lead time for the firm statement to go from a warning to an error. I threw out 3 months as a starting point for discussion; I don't think it can reasonably be any less than that. Upgrades like this need to be tested, deployed to lots of people and bots, issues found and corrected. These things take time.
+wolfgang who has been working in this area also
+echristo re defining an LLVM range of language codes.
It would be safer if we came up with an LLVM extension range for language constants (assuming the DWARF committee doesn't change the process for assigning language constants). It will be years before DWARF v6 comes out and there is a very reasonable chance that the numbers won't match.
I'd rather we don't ship any LLVM that could emit an incorrect language code.
Oct 3 2018
Oct 1 2018
Sep 27 2018
What I meant by "use case" is, do you have a front end that will make use of these new features?
Sep 26 2018
clang-format-diff is your friend.
Sep 21 2018
Do we generate the .dwo file directly these days? If not, I can imagine wanting to avoid the overhead of the objcopy hack; as long as the linker is smart enough not to bother with the .debug_*.dwo sections this seems like a build-time win.
Sep 19 2018
Sep 18 2018
Sep 17 2018
Sep 14 2018
Sep 13 2018
Sep 12 2018
Can you preserved the declaration DIE (at 0x0000007c) while omitting the definition DIE (at 0x00000063)? Does that still cause a problem for GDB?
Sep 4 2018
Aug 28 2018
Aug 27 2018
The indentation in the test looks a little odd. Please verify there are no hard tabs.
Aug 22 2018
Aug 10 2018
Aug 8 2018
Aug 7 2018
I'm abandoning this minor tweak as I am convinced that there is something more fundamentally wrong here.
llvm-dwarfdump doesn't call the right API sequence to show the bug; it requires a DWP, looking up a specific unit by hash via the DWP index, and then scanning all the units. I can envision how to do a unittest for that, but the unittest infrastructure doesn't currently support building a DWP. I can't take the time right now to address this, but I'll keep it on The List.