Skip to content

8316557: Make fields final in 'sun.util' package #15736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
turbanoff wants to merge 3 commits into from
Closed

8316557: Make fields final in 'sun.util' package #15736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
26d00dc
[PATCH] Make fields final in 'sun.util' package
turbanoff Sep 14, 2023
5db7410
8316557: Make fields final in 'sun.util' package
turbanoff Nov 16, 2023
2c9b4ed
8316557: Make fields final in 'sun.util' package
turbanoff Nov 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/java.base/share/classes/sun/util/PreHashedMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2004, 2012, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2004, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -243,7 +243,7 @@ public boolean hasNext() {

public Map.Entry<String,V> next() {
return new Map.Entry<String,V>() {
String k = i.next();
final String k = i.next();
public String getKey() { return k; }
public V getValue() { return get(k); }
public int hashCode() {
Expand Down
13 changes: 4 additions & 9 deletions src/java.base/share/classes/sun/util/PropertyResourceBundleCharset.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -35,7 +35,6 @@
import java.nio.charset.CharsetEncoder;
import java.nio.charset.CoderResult;
import java.nio.charset.CodingErrorAction;
import java.nio.charset.StandardCharsets;
import java.util.Objects;

/**
Expand All @@ -45,17 +44,13 @@
*/
public class PropertyResourceBundleCharset extends Charset {

private boolean strictUTF8 = false;
private final boolean strictUTF8;

public PropertyResourceBundleCharset(boolean strictUTF8) {
this(PropertyResourceBundleCharset.class.getCanonicalName(), null);
super(PropertyResourceBundleCharset.class.getCanonicalName(), null);
this.strictUTF8 = strictUTF8;
}

public PropertyResourceBundleCharset(String canonicalName, String[] aliases) {
super(canonicalName, aliases);
}

@Override
public boolean contains(Charset cs) {
return false;
Expand All @@ -73,7 +68,7 @@ public CharsetEncoder newEncoder() {

private final class PropertiesFileDecoder extends CharsetDecoder {

private CharsetDecoder cdUTF_8 = UTF_8.INSTANCE.newDecoder()
private final CharsetDecoder cdUTF_8 = UTF_8.INSTANCE.newDecoder()
Copy link
Member

@liach liach Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be static as well.

Copy link
Member Author

@turbanoff turbanoff Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. Why do you think so? CharsetDecoder is not thread-safe.

Copy link
Member

@liach liach Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UTF8 decoder does not perform any internal state mutation during decoding; in addition, if this field is static final, the decoder's error actions are published when the class is initialized, while publishing in a final field does not guarantee the internal state of the charset is visible to other threads.

Copy link
Contributor

@Glavo Glavo Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UTF8 decoder does not perform any internal state mutation during decoding;

Are you sure? I think CharsetDecoder::decode will modify the status field.

Copy link
Member

@liach liach Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, it does have a state field.

Copy link
Contributor

@AlanBateman AlanBateman Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CharsetDecoder is specified to be not safe for use by multiple concurrent threads, would be too fragile to assume behavior of a specific implementation.

.onMalformedInput(CodingErrorAction.REPORT)
.onUnmappableCharacter(CodingErrorAction.REPORT);
private CharsetDecoder cdISO_8859_1 = null;
Expand Down
8 changes: 4 additions & 4 deletions src/java.base/share/classes/sun/util/ResourceBundleEnumeration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2005, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -36,9 +36,9 @@
*/
public class ResourceBundleEnumeration implements Enumeration<String> {

Set<String> set;
Iterator<String> iterator;
Enumeration<String> enumeration; // may remain null
private final Set<String> set;
private final Iterator<String> iterator;
private final Enumeration<String> enumeration; // may remain null

/**
* Constructs a resource bundle enumeration.
Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/sun/util/calendar/LocalGregorianCalendar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ private static boolean isValidEra(Era newEra, Era[] eras) {
return true;
}

private String name;
private Era[] eras;
private final String name;
private final Era[] eras;

public static class Date extends BaseCalendar.Date {

Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/sun/util/calendar/ZoneInfoFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ private ZoneInfoFile() {

private static String versionId;
private static final Map<String, ZoneInfo> zones = new ConcurrentHashMap<>();
private static Map<String, String> aliases = new HashMap<>();
private static final Map<String, String> aliases = new HashMap<>();

private static byte[][] ruleArray;
private static String[] regions;
Expand All @@ -219,7 +219,7 @@ private ZoneInfoFile() {
// Flag for supporting JDK backward compatible IDs, such as "EST".
private static final boolean USE_OLDMAPPING;

private static String[][] oldMappings = new String[][] {
private static final String[][] oldMappings = new String[][] {
{ "ACT", "Australia/Darwin" },
{ "AET", "Australia/Sydney" },
{ "AGT", "America/Argentina/Buenos_Aires" },
Expand Down
8 changes: 3 additions & 5 deletions src/java.base/share/classes/sun/util/cldr/CLDRCalendarDataProviderImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -27,14 +27,12 @@

import static sun.util.locale.provider.LocaleProviderAdapter.Type;

import java.util.Arrays;
import java.util.Map;
import java.util.Locale;
import java.util.Set;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import sun.util.locale.provider.LocaleProviderAdapter;
import sun.util.locale.provider.LocaleResources;
import sun.util.locale.provider.CalendarDataProviderImpl;
import sun.util.locale.provider.CalendarDataUtility;

Expand All @@ -47,8 +45,8 @@
*/
public class CLDRCalendarDataProviderImpl extends CalendarDataProviderImpl {

private static Map<String, Integer> firstDay = new ConcurrentHashMap<>();
private static Map<String, Integer> minDays = new ConcurrentHashMap<>();
private static final Map<String, Integer> firstDay = new ConcurrentHashMap<>();
Copy link
Contributor

@minborg minborg Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might rename these fields to FIRST_DAY etc.

Copy link
Member Author

@turbanoff turbanoff Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. Usually CAPS naming is used only on immutable constants, but here we have mutable field. (See google style guide for example - https://google.github.io/styleguide/javaguide.html#s5.2.4-constant-names).
I like old naming approach.

private static final Map<String, Integer> minDays = new ConcurrentHashMap<>();

public CLDRCalendarDataProviderImpl(Type type, Set<String> langtags) {
super(type, langtags);
Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -60,7 +60,7 @@ public class CLDRLocaleProviderAdapter extends JRELocaleProviderAdapter {
private final LocaleDataMetaInfo nonBaseMetaInfo;

// parent locales map
private static volatile Map<Locale, Locale> parentLocalesMap;
private static final Map<Locale, Locale> parentLocalesMap;
Copy link
Contributor

@minborg minborg Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to PARENT_LOCALES_MAP?

// cache to hold locale to locale mapping for language aliases.
private static final Map<Locale, Locale> langAliasesCache;
// cache the available locales
Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/sun/util/locale/LocaleObjectCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -100,7 +100,7 @@ protected K normalizeKey(K key) {
}

private static class CacheEntry<K, V> extends SoftReference<V> {
Copy link
Contributor

@minborg minborg Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class itself might be final

Copy link
Member Author

@turbanoff turbanoff Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think making classes final is out of scope of this PR.

private K key;
private final K key;

CacheEntry(K key, V value, ReferenceQueue<V> queue) {
super(value, queue);
Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/sun/util/locale/LocaleSyntaxException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -36,7 +36,7 @@ public class LocaleSyntaxException extends Exception {
@java.io.Serial
private static final long serialVersionUID = 1L;

private int index = -1;
private final int index;

public LocaleSyntaxException(String msg) {
this(msg, 0);
Expand Down
16 changes: 6 additions & 10 deletions src/java.base/share/classes/sun/util/locale/StringTokenIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2011, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -31,9 +31,9 @@
package sun.util.locale;

public class StringTokenIterator {
Copy link
Contributor

@minborg minborg Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class may be final

private String text;
private String dlms; // null if a single char delimiter
private char delimiterChar; // delimiter if a single char delimiter
private final String text;
private final String dlms; // null if a single char delimiter
private final char delimiterChar; // delimiter if a single char delimiter

private String token;
private int start;
Expand All @@ -44,7 +44,9 @@ public StringTokenIterator(String text, String dlms) {
this.text = text;
if (dlms.length() == 1) {
delimiterChar = dlms.charAt(0);
this.dlms = null;
} else {
delimiterChar = 0;
this.dlms = dlms;
}
setStart(0);
Expand Down Expand Up @@ -99,12 +101,6 @@ public StringTokenIterator setStart(int offset) {
return this;
}

public StringTokenIterator setText(String text) {
this.text = text;
setStart(0);
return this;
}

private int nextDelimiter(int start) {
int textlen = this.text.length();
if (dlms == null) {
Expand Down