This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] libdw: Correct spelling of DW_LANG_PLI in dwarf.h.


On Wed, 2016-10-26 at 11:18 -0700, Josh Stone wrote:
> On 10/26/2016 10:47 AM, Mark Wielaard wrote:
> > On Wed, Oct 26, 2016 at 09:36:31AM -0700, Josh Stone wrote:
> >> On 10/26/2016 03:24 AM, Mark Wielaard wrote:
> >>> The name used in the standard and other DWARF implementations for
> >>> Programming Language One, PL/I, is DW_LANG_PLI (not DW_LANG_PL1).
> >>> [...]
> >>> +Version 0.168
> >>> +
> >>> +libdw: dwarf.h corrected the DW_LANG_PLI constant name (was DW_LANG_PL1).
> >>> +       Any existing sources using the old name will have to be updated.
> >>> +
> >>
> >> That typo has been there since the dawn of elfutils.git, 2005-07-26
> >> commit b08d5a8fb42f.  It seems callous to break API here, even if it was
> >> wrong.  Why not include a #define to preserve compatibility?
> > 
> > Since it doesn't break ABI we have historically just fixed such wrong
> > constant names, see e.g. the removal in 0.160 of the non-existing
> > DW_TAG_mutable_type in 0.160. Given that others (libdwarf, binutils)
> > do define the correct name in their headers.
> 
> It doesn't matter what other headers do -- if someone was already using
> the bad constant in elfutils' headers, that's an API break.  But sure,
> you might still decide that's acceptable.

No, you are probably right that isn't acceptable. The case is different
from DW_TAG_mutable_type, which didn't have a real meaning/use. Lets go
with a compatibility #define DW_LANG_PL1 DW_LANG_PLI.

> >>> -    DW_LANG_PL1 = 0x000f,	     /* ISO PL/1:1976 */
> >>> +    DW_LANG_PLI = 0x000f,	     /* ISO PL/1:1976 */
> >>
> >> You should correct the comment too, "PL/I".
> > 
> > The '1' in the comment really is correct, because that
> > is the actual name of the standard. That is probably where
> > the confusion came from.
> 
> Figure 8 in the DWARF 3 and 4 standards both call it "ANSI PL/I:1976"
> (not "ISO"), as does Table 3.1 in the DWARF 5 draft I've got.
> 
> Wikipedia also uses "PL/I", and even has a redirect from "PL/1".
> 
> This "I" is pronounced "one" though, surely as a roman numeral.

ISO/IEC 6522:1992 "Adopts ANSI standard X3.53-1976 [...] as PL/1
programming language standard". Lets keep it as PL/1 to at least show
where the confusion came from.

Updated patch attached.

Cheers,

Mark
From a353cd53e48e61282fa5daf878fb3863750d1287 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Sat, 22 Oct 2016 19:18:51 +0200
Subject: [PATCH] libdw: Correct spelling of DW_LANG_PLI in dwarf.h.

The name used in the standard and other DWARF implementations for
Programming Language One, PL/I, is DW_LANG_PLI (not DW_LANG_PL1).

Fix usage in dwarf_aggregate_size.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 NEWS                         | 4 ++++
 libdw/ChangeLog              | 8 +++++++-
 libdw/dwarf.h                | 6 ++++--
 libdw/dwarf_aggregate_size.c | 4 ++--
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 599970b..b2d2ffa 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+Version 0.168
+
+libdw: dwarf.h corrected the DW_LANG_PLI constant name (was DW_LANG_PL1).
+
 Version 0.167
 
 libasm: Add eBPF disassembler for EM_BPF files.
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index c2f25ea..5c2968b 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,4 +1,10 @@
-2015-10-11  Akihiko Odaki  <akihiko.odaki.4i@stu.hosei.ac.jp>
+2016-10-22  Mark Wielaard  <mjw@redhat.com>
+
+	* dwarf.h: Correct spelling of DW_LANG_PLI. Add compatibility define.
+	* dwarf_aggregate_size.c (array_size): Use correct spelling of
+	DW_LANG_PLI.
+
+2016-10-11  Akihiko Odaki  <akihiko.odaki.4i@stu.hosei.ac.jp>
 
 	* dwarf_getpubnames.c: Remove sys/param.h include, add system.h.
 	* libdw_alloc.c: Likewise.
diff --git a/libdw/dwarf.h b/libdw/dwarf.h
index b5c58d7..169b53e 100644
--- a/libdw/dwarf.h
+++ b/libdw/dwarf.h
@@ -1,5 +1,5 @@
 /* This file defines standard DWARF types, structures, and macros.
-   Copyright (C) 2000-2011, 2014 Red Hat, Inc.
+   Copyright (C) 2000-2011, 2014, 2016 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -582,7 +582,7 @@ enum
     DW_LANG_C99 = 0x000c,	     /* ISO C:1999 */
     DW_LANG_Ada95 = 0x000d,	     /* ISO Ada:1995 */
     DW_LANG_Fortran95 = 0x000e,	     /* ISO Fortran 95 */
-    DW_LANG_PL1 = 0x000f,	     /* ISO PL/1:1976 */
+    DW_LANG_PLI = 0x000f,	     /* ISO PL/1:1976 */
     DW_LANG_ObjC = 0x0010,	     /* Objective-C */
     DW_LANG_ObjC_plus_plus = 0x0011, /* Objective-C++ */
     DW_LANG_UPC = 0x0012,	     /* Unified Parallel C */
@@ -602,6 +602,8 @@ enum
     DW_LANG_hi_user = 0xffff
   };
 
+/* Old (typo) '1' != 'I'.  */
+#define DW_LANG_PL1 DW_LANG_PLI
 
 /* DWARF identifier case encodings.  */
 enum
diff --git a/libdw/dwarf_aggregate_size.c b/libdw/dwarf_aggregate_size.c
index aaeb7ed..52ef006 100644
--- a/libdw/dwarf_aggregate_size.c
+++ b/libdw/dwarf_aggregate_size.c
@@ -1,5 +1,5 @@
 /* Compute size of an aggregate type from DWARF.
-   Copyright (C) 2010, 2014 Red Hat, Inc.
+   Copyright (C) 2010, 2014, 2016 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -127,7 +127,7 @@ array_size (Dwarf_Die *die, Dwarf_Word *size,
 		    case DW_LANG_Fortran08:
 		    case DW_LANG_Pascal83:
 		    case DW_LANG_Modula2:
-		    case DW_LANG_PL1:
+		    case DW_LANG_PLI:
 		      lower = 1;
 		      break;
 
-- 
1.8.3.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]