RFR(XS): 8169734: Update uses of string "java.base" to macro

[Daniel D. Daugherty]

On 12/5/16 9:41 AM, Rachel Protacio wrote:
> Thanks, Dan! Ok, here's the webrev with all the instances changed. 
> http://cr.openjdk.java.net/~rprotacio/8169734.01/

src/share/vm/classfile/classLoader.cpp
     No comments.

src/share/vm/classfile/javaClasses.cpp
     No comments.

src/share/vm/classfile/moduleEntry.cpp
     No comments.

src/share/vm/classfile/modules.cpp
     No comments.

src/share/vm/classfile/packageEntry.cpp
     No comments.

src/share/vm/classfile/vmSymbols.hpp
     No comments.

src/share/vm/oops/arrayKlass.cpp
     No comments.

src/share/vm/oops/instanceKlass.cpp
     No comments.

src/share/vm/runtime/arguments.cpp
     No comments.

Thumbs up!

I did not do a "completeness" check. I presume that Serguei will
do that when he re-reviews.

Dan


>
> Rachel
>
>
> On 12/2/2016 4:18 PM, Daniel D. Daugherty wrote:
>> > I'd suggest to collect at least one more opinion/review on this.
>>
>> Let's get this correct now and not have to come back and fix it down
>> the road. Looks to me like these new locations can take advantage
>> of C++ string concatenation unless I'm missing something, e.g.:
>>
>> 775              "Incorrect boot loader search path, no java runtime 
>> image or java.base exploded build");
>>
>> becomes:
>>
>> 775              "Incorrect boot loader search path, no java runtime 
>> image or " JAVA_BASE " exploded build");
>>
>> which is a bit long, but not too hard to deal with... And since we're
>> migrating away from an informal limit of 80 chars per line... I won't
>> even complain... :-)
>>
>> Dan
>>
>>
>> On 12/2/16 1:52 PM, serguei.spitsyn at oracle.com wrote:
>>> Hi Rachel,
>>>
>>>
>>> On 12/2/16 11:12, Rachel Protacio wrote:
>>>> Hi,
>>>>
>>>> Thanks for the review. Yeah, we had thought about that. Two 
>>>> concerns: (1) the concatenation or %s is a little verbose and 
>>>> possibly excessive for these ancillary error messages, and (2) 
>>>> because they are error messages, potentially someone could 
>>>> hard-code something to expect the output and if for some reason we 
>>>> change the macro in the future it could throw people off 
>>>> unnecessarily... Just some thoughts.
>>>
>>> Yes, I was thinking about the same.
>>>
>>>
>>>> I'm happy to change all of them though if that's what we want.
>>>
>>> I'd suggest to collect at least one more opinion/review on this.
>>> It is Ok with me to keep it as it is.
>>>
>>>
>>>> I'll definitely change the classfile/vmSymbols.hpp and 
>>>> runtime/os.cpp instances.
>>>
>>> Thanks!
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>> Let me know what you think!
>>>>
>>>> Rachel
>>>>
>>>> On 12/1/2016 5:43 PM, serguei.spitsyn at oracle.com wrote:
>>>>> Hi Rachel,
>>>>>
>>>>> It looks good in general.
>>>>> However, there are more instances of java.base that may need an 
>>>>> update.
>>>>>
>>>>> classfile/classLoader.cpp:
>>>>>
>>>>>  775              "Incorrect boot loader search path, no java 
>>>>> runtime image or java.base exploded build");
>>>>>
>>>>> 1826       vm_exit_during_initialization("Unable to create 
>>>>> ModuleEntry for java.base");
>>>>>
>>>>>
>>>>> classfile/modules.cpp:
>>>>>
>>>>>  180                 "Bad package name for module: java.base");
>>>>>
>>>>>  185                 err_msg("Invalid package name: %s for module: 
>>>>> java.base", package_name));
>>>>>
>>>>>  191                 err_msg("Duplicate package name: %s for 
>>>>> module java.base",
>>>>>
>>>>>  209   assert(ModuleEntryTable::javabase_moduleEntry() != NULL, 
>>>>> "No ModuleEntry for java.base");
>>>>>
>>>>>  230           assert(pkg != NULL, "Unable to create a java.base 
>>>>> package entry");
>>>>>
>>>>>  244               "Module java.base is already defined");
>>>>>
>>>>>  253   log_debug(modules)("define_javabase_module(): Definition of 
>>>>> module: java.base,"
>>>>>
>>>>>  261     log_trace(modules)("define_javabase_module(): creation of 
>>>>> package %s for module java.base",
>>>>>
>>>>>  716   assert(ModuleEntryTable::javabase_defined(), "Attempt to 
>>>>> call get_module before java.base is defined");
>>>>>
>>>>>  762          "Attempt to call get_module_from_pkg before 
>>>>> java.base is defined");
>>>>>
>>>>>  799          "Attempt to call get_named_module before java.base 
>>>>> is defined");
>>>>>
>>>>>
>>>>> runtime/arguments.cpp:
>>>>>
>>>>> 3432       vm_exit_during_initialization("Cannot specify java.base 
>>>>> more than once to --patch-module");
>>>>>
>>>>>
>>>>> Some other files have it too:
>>>>>
>>>>> classfile/vmSymbols.hpp: template(java_base, 
>>>>> "java.base")                                \
>>>>> classfile/moduleEntry.cpp:    fatal("Unable to finalize module 
>>>>> definition for java.base");
>>>>> classfile/moduleEntry.cpp:  assert(jb_module != NULL, "java.base 
>>>>> ModuleEntry not defined");
>>>>> classfile/moduleEntry.cpp:    fatal("Unable to patch the module 
>>>>> field of classes loaded prior to java.base's definition, invalid 
>>>>> java.lang.reflect.Module");
>>>>> classfile/packageEntry.cpp: vm_exit_during_initialization("A 
>>>>> non-java.base package was loaded prior to module system 
>>>>> initialization", entry->name()->as_C_string());
>>>>> oops/arrayKlass.cpp:         "module entry not available post 
>>>>> java.base definition");
>>>>> oops/instanceKlass.cpp: 
>>>>> assert(ModuleEntryTable::javabase_moduleEntry() != NULL, 
>>>>> "java.base module is NULL");
>>>>> runtime/os.cpp:  char* base_classes = 
>>>>> format_boot_path("%/modules/java.base", home, home_len, fileSep, 
>>>>> pathSep);
>>>>>
>>>>> So, the question is if it's worth to update the remaining 
>>>>> instances as well.
>>>>> Simple concatenation or %s replacement in some rare cases could be 
>>>>> used for it.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>> On 12/1/16 13:20, Rachel Protacio wrote:
>>>>>> Hi!
>>>>>>
>>>>>> Please review this little change, replacing errant uses of the 
>>>>>> string "java.base" with the macro JAVA_BASE_NAME. Passes JPRT.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8169734
>>>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8169734.00/
>>>>>>
>>>>>> Thanks,
>>>>>> Rachel
>>>>>
>>>>
>>>
>>>
>>
>