[foreign] RFR 8222288: jextract generates unescaped String literals for macros

[Maurizio Cimadamore]

Looks good

Maurizio

On 11/04/2019 06:53, Sundararajan Athijegannathan wrote:
> Good catch! Updated: 
> http://cr.openjdk.java.net/~sundar/8222288/webrev.01/index.html
>
> PS.
>
> * Build/tests fine on all platform (via mach5).
> * I guess my earlier patch also passed on Windows. It seems that 
> repeated backslash (\\) in the path name (while redundant) is accepted.
>
> Thanks,
> -Sundar
>
> On 10/04/19, 11:10 PM, Jorn Vernee wrote:
>> This patch also makes my earlier fix for paths on Windows redundant 
>> [1], since now the problem is taken care of when emitting the string 
>> instead.
>>
>> You could remove the old changes to JavaSourceFactory.java at this 
>> point. I've already tested that this works as well:
>>
>> diff -r a4510fd757fc 
>> src/jdk.jextract/share/classes/com/sun/tools/jextract/JavaSourceFactory.java
>> --- 
>> a/src/jdk.jextract/share/classes/com/sun/tools/jextract/JavaSourceFactory.java 
>>      Wed Apr 10 19:22:59 2019 +0200
>> +++ 
>> b/src/jdk.jextract/share/classes/com/sun/tools/jextract/JavaSourceFactory.java 
>>      Wed Apr 10 19:34:04 2019 +0200
>> @@ -99,7 +99,7 @@
>>          global_jsb.addPackagePrefix(headerFile.pkgName);
>>
>>          Map<String, Object> header = new HashMap<>();
>> -        header.put("path", 
>> headerFile.path.toAbsolutePath().toString().replace("\\", "\\\\"));
>> +        header.put("path", 
>> headerFile.path.toAbsolutePath().toString());
>>          if (!libraryNames.isEmpty()) {
>>              header.put("libraries", libraryNames.toArray(new 
>> String[0]));
>>              if (libraryPaths != null && !libraryPaths.isEmpty()) {
>> @@ -168,7 +168,7 @@
>>              SourceLocation.Location loc = src.getFileLocation();
>>              Path p = loc.path();
>>              Map<String, Object> fields = new HashMap<>();
>> -            fields.put("file", p == null ? "<builtin>" : 
>> p.toAbsolutePath().toString().replace("\\", "\\\\"));
>> +            fields.put("file", p == null ? "<builtin>" : 
>> p.toAbsolutePath().toString());
>>              fields.put("line", loc.line());
>>              fields.put("column", loc.column());
>>              jsb.addAnnotation(align, NATIVE_LOCATION, fields);
>>
>> Cheers,
>> Jorn
>>
>> [1] : http://hg.openjdk.java.net/panama/dev/rev/1daf5d6e017c
>>
>> Sundararajan Athijegannathan schreef op 2019-04-10 19:02:
>>> Please review.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8222288
>>> Webrev: https://cr.openjdk.java.net/~sundar/8222288/webrev.00/
>>>
>>> PS. Thanks to Maurizio for suggestion on javac source for 'source
>>> safe' String escape!
>>>
>>> Thanks,
>>> -Sundar