On 01/16/18 17:13, David Gibson wrote: > On Mon, Jan 15, 2018 at 07:24:10PM +0100, Julia Lawall wrote: >> This commit provides two new command-line options: >> >> --annotate (abbreviated -T) >> --annotate-full (abbreviated -F) > > What's the rationale for having two different versions of the > annotations? I'll put an example to try to explain, at the end of this email. >> >> --annotate provides one or more filenames and line numbers indicating >> the origin of a given line. The filename is expressed relative the the >> filename provided on the command line. Nothing is printed for overlays, >> etc. >> >> --annotate-full provides one or more tuples of: filename, starting line, >> starting column, ending line ending column. The full path is given for >> the file name. Overlays, etc are annotated with <no-file>:<no-line>. >> >> There are numerous changes in srcpos to provide the relative filenames >> (variables initial_path, initial_pathlen and initial_cpp, new functions >> set_initial_path and shorten_to_initial_path, and changes in >> srcfile_push and srcpos_set_line). The change in srcpos_set_line takes >> care of the case where cpp is used as a preprocessor. In that case the >> initial file name is not the one provided on the command line but the >> one found at the beginnning of the cpp output. >> >> The new functions srcpos_string_comment, srcpos_string_first, and >> srcpos_string_last print the annotations. srcpos_string_comment is >> recursive to print a list of source file positions. >> >> Note that the column numbers in the --annotate-full case may be of >> limited use when cpp is first used, because the columns are those in the >> output generated by cpp, which are not the same as the ones in the >> source code. It may be that cpp simply replaces tabs by spaces. >> >> Various changes are sprinkled throughout treesource.c to print the >> annotations. >> >> Signed-off-by: Julia Lawall <Julia.Lawall@xxxxxxx> >> Signed-off-by: Frank Rowand <frank.rowand@xxxxxxxx> < snip > For a real world Linux devicetree source file, I often use arch/arm/boot/dts/qcom-apq8074-dragonboard.dts In these examples, I am using the version in Linux v4.15-rc7. My basic premise is that --annotate-full provides a lot of useful information, but that information is usually not needed. But the extra information makes the output very dense and much harder for a human eye to scan or read. In the Linux sources (an argument _against_ my case :-)), the full path of the source files normally provides no value. Part of this the way our source tree is structured. I will use the Linux script scripts/dtc/dtx_diff to generate my examples of annotated files. This script takes care of all of the include paths, invoking cpp, and other details of the linux use of dtc. When provided a single argument, dtx_diff simply does the compile with '-O dts'. I have modified the normal dtx_diff to pass on --annotate and --annotate-full to dtc. First, I create two annotated files, one with --annotate, the other with --annotate-full: $ scripts/dtc/dtx_diff --annotate arch/arm/boot/dts/qcom-apq8074-dragonboard.dts >qcom-apq8074-dragonboard--annotate.dts $ scripts/dtc/dtx_diff --annotate-full arch/arm/boot/dts/qcom-apq8074-dragonboard.dts >qcom-apq8074-dragonboard--annotate-full.dts Then just look at the first few lines of each of the two examples: $ head qcom-apq8074-dragonboard--annotate.dts /dts-v1/; / { /* qcom-apq8074-dragonboard.dts:6, qcom-msm8974.dtsi:11, skeleton.dtsi:12 */ #address-cells = <0x1>; /* skeleton.dtsi:13 */ #size-cells = <0x1>; /* skeleton.dtsi:14 */ compatible = "qcom,apq8074-dragonboard", "qcom,apq8074"; /* qcom-apq8074-dragonboard.dts:8 */ interrupt-parent = <0x1>; /* qcom-msm8974.dtsi:14 */ model = "Qualcomm APQ8074 Dragonboard"; /* qcom-apq8074-dragonboard.dts:7 */ adsp-pil { /* qcom-msm8974.dtsi:243 */ $ head qcom-apq8074-dragonboard--annotate-full.dts /dts-v1/; / { /* arch/arm/boot/dts/qcom-apq8074-dragonboard.dts:6:0-346:2, arch/arm/boot/dts/qcom-msm8974.dtsi:11:0-1148:2, arch/arm/boot/dts/skeleton.dtsi:12:0-18:2 */ #address-cells = <0x1>; /* arch/arm/boot/dts/skeleton.dtsi:13:1-13:22 */ #size-cells = <0x1>; /* arch/arm/boot/dts/skeleton.dtsi:14:1-14:19 */ compatible = "qcom,apq8074-dragonboard", "qcom,apq8074"; /* arch/arm/boot/dts/qcom-apq8074-dragonboard.dts:8:1-8:57 */ interrupt-parent = <0x1>; /* arch/arm/boot/dts/qcom-msm8974.dtsi:14:1-14:28 */ model = "Qualcomm APQ8074 Dragonboard"; /* arch/arm/boot/dts/qcom-apq8074-dragonboard.dts:7:1-7:40 */ adsp-pil { /* arch/arm/boot/dts/qcom-msm8974.dtsi:243:1-262:3 */ The --annotate-full version is harder for _me_ to scan. The --annotate-full version results in a much longer line (and this effect gets even worse when there are nodes nested several levels). One can make the argument that the long path is the fault of the way that Linux is structured, and it should be my problem to resolve that. And to a certain extent I can. I could package a simple sed command in a script. The simplistic hard coded version of the sed command (since I know the base path of the devicetree source file that I am compiling) is: $ sed -e 's|arch/arm/boot/dts/||g' -e 's|:[0-9]*-[0-9]*:[0-9]*||g' -e 's| /\* <no-file>:<no-line> \*/||' qcom-apq8074-dragonboard--annotate-full.dts > qcom-apq8074-dragonboard--annotate-full--then-sed.dts The second "-e" substitution is not needed to strip the path off, but I was going for the full simplification. Looking at the first few lines of that transformation, I see that it is basically the same as using --annotate. Looking deeper into the file, which I don't show here, some of the row numbers are different, due to my simplistic sed regular expression. $ head qcom-apq8074-dragonboard--annotate-full--then-sed.dts /dts-v1/; / { /* qcom-apq8074-dragonboard.dts:6, qcom-msm8974.dtsi:11, skeleton.dtsi:12 */ #address-cells = <0x1>; /* skeleton.dtsi:13 */ #size-cells = <0x1>; /* skeleton.dtsi:14 */ compatible = "qcom,apq8074-dragonboard", "qcom,apq8074"; /* qcom-apq8074-dragonboard.dts:8 */ interrupt-parent = <0x1>; /* qcom-msm8974.dtsi:14 */ model = "Qualcomm APQ8074 Dragonboard"; /* qcom-apq8074-dragonboard.dts:7 */ adsp-pil { /* qcom-msm8974.dtsi:243 */ So I could achieve the equivalent of --annotate by post-processing, possible with the result of slightly different line numbers if I'm sloppy. But if dtc can do the simplification, I don't need to add the additional external processing. The question of whether to include just the starting line number, or the starting line/column and ending line/column was the trade off between more information vs less dense information being easier to scan and read. Having both --annotate and --annotate-full lets the user make the trade off as needed in any particular case. -Frank -- To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html