On Jul 11, 2012, at 12:01 AM, Michael Haupt wrote:
> @@ -1636,16 +1648,163 @@
> The code for parsing @Retention deserves a comment highlighting that it is
> about parsing an annotation with payload (none of the annotations introduced
> by our work do this).
>
> @@ -2560,10 +2727,11 @@
> TempNewSymbol sde_symbol is never used.
Fixed; thanks.
On Jul 11, 2012, at 2:35 PM, Vladimir Kozlov wrote:
> c1_GraphBuilder.cpp, can you remove setting bailout message for forced
> inlining? It should be done proper uniform way for C1 inlining later.
Done.
> I think, next assert should check (id >= _unknown && id < _annotation_LIMIT)
> instead:
>
> + void set_annotation(ID id) {
> + assert((int)id > 0 && (int)id < BitsPerInt, "oob");
Good. I added this to the constructor
assert((int)_annotation_LIMIT <= (int)sizeof(_annotations_present) *
BitsPerByte, "");
> In header file classFileParser.hpp you should not specify ClassFileParser::
> in method parse_classfile_attributes(() declaration:
>
> ClassFileParser::ClassAnnotationCollector* parsed_annotations
Good catch.
> Instead of asserts in apply_to() methods we should use guarantee("not
> implemented") or something.
Done:
+ guarantee(false, "no field annotations yet");
>
> I don't think next should be part of these changes:
>
> +#if 0
> + // The parsing of @Retention is for example only.
Yes, Michael asked about that too. The point is to show how annotation
payloads can be processed. Somebody will need it at some point. I'll make it
into a comment; is that OK?
+ // For the record, here is how annotation payloads can be collected.
+ // Suppose we want to capture @Retention.value. Here is how:
+ //if (id == AnnotationCollector::_class_Retention) {
> Add parenthesis around expression in next condition:
>
> + while (--nann >= 0 && index-2 + min_size <= limit) {
Done:
+ while ((--nann) >= 0 && (index-2 + min_size <= limit)) {
> Instead of passing pointers to classFileParser's new attributes fields
> (_synthetic_flag, _sourcefile, ...) as arguments add accessors functions
> which set these fields.
The pointer passing is following the pattern already present for parsing other
constructs. parse_fields is the clearest example. The parsed information is
returned by reference. I could do as you suggest, but it would work only for
top-level class attributes, so there would still be a mix of styles. My
thought is to make the style more uniform by relying on the return-by-reference
pattern.
But I'll change it if you insist.
> I think next is typo, should be _in_method check:
>
> + case
> vmSymbols::VM_SYMBOL_ENUM_NAME(java_lang_invoke_ForceInline_signature):
> + if (_location != _in_class) break;
> + return _method_ForceInline;
> + default: break;
> + }
Yes; thanks.
— John_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
