Extend Annotation Support #1125

iguessthislldo · 2020-05-29T06:40:19Z

Annotation support extended to on the following types and their
contents:

Interfaces
Porttypes
Eventtypes
Components

Annotations on Valuetypes and most of their possible contents are also
supported. The exceptions to this are these types of valuetype
statements:

import: not supported by TAO
typeid: not supported by TAO on valuetypes
typeprefix: No corresponding AST Node to attach annotations to

Annotation support extended to on the following types and their contents: - Interfaces - Porttypes - Eventtypes - Components Annotations on Valuetypes and most of their possible contents are also supported. The exceptions to this are these types of valuetype statements: - import: not supported by TAO - typeid: not supported by TAO on valuetypes - typeprefix: No corresponding AST Node to attach annotations to

iguessthislldo · 2020-05-29T17:09:47Z

The errors in VS2017 WChar aren't the same as on master, but they're similar. I feel like I've seen this behavior in TAO before where the build fails because the results from IDL weren't generated in the proper order.

TAO/TAO_IDL/docs/annotations.md

TAO/TAO_IDL/fe/idl.ypp

TAO/TAO_IDL/docs/annotations.md

okellogg · 2020-05-30T10:11:50Z

By the way, TAO/tests/IDLv4/TestIDLv4.idl can be turned into valid IDLv4 with these changes:

diff --git a/TAO/tests/IDLv4/TestIDLv4.idl b/TAO/tests/IDLv4/TestIDLv4.idl
index d46d65705e7..614536616df 100644
--- a/TAO/tests/IDLv4/TestIDLv4.idl
+++ b/TAO/tests/IDLv4/TestIDLv4.idl
@@ -77,6 +77,8 @@ struct TestAnon {
 
 // 7.4.15.4.2 Applying Annotations
 
+const double dub = 6.78;
+
 @abc
 struct Annotated1 {
 
@@ -89,11 +91,10 @@ struct Annotated1 {
   @aa(L"bbcc") octet g;
   @ddee(4 + 5) octet h;
 
-  const double dub = 6.78;
   @ffgg(dub) octet i;
 
   @hhii(jj = FALSE) octet j;
-  @kkll(mm = 3, nn = 'oo') octet k;
+  @kkll(mm = 3, nn = 'o') octet k;
 };
 
 @bit_bound(8)

Okay to piggyback on here, or do you prefer a separate pull request?

mitza-oci · 2020-05-30T13:31:21Z

Okay to piggyback on here, or do you prefer a separate pull request?

Let's do that one separately. It's not related to the changes in this PR.

jwillemsen · 2020-05-31T08:37:48Z

@iguessthislldo Looks this PR causes some compile and runtime issues, see for example https://buildlogs.remedy.nl/rhel70_atcd_debug_ndds523rev0/index.html

[Details] /home/build/jenkins/workspace/rhel70_atcd_debug_ndds523rev0/BUILD/DOC_ROOT/TAO/tests/IDLv4/annotations/Annotation_Test.h: In instantiation of ‘T* Annotation_Test::assert_node(const char*, UTL_Scope*) [with T = AST_Annotation_Decl]’:
[Details] /home/build/jenkins/workspace/rhel70_atcd_debug_ndds523rev0/BUILD/DOC_ROOT/TAO/tests/IDLv4/annotations/Annotation_Test.cpp:201:69: required from here
[Details] /home/build/jenkins/workspace/rhel70_atcd_debug_ndds523rev0/BUILD/DOC_ROOT/TAO/tests/IDLv4/annotations/Annotation_Test.h:64:14: error: cannot pass objects of non-trivially-copyable type ‘std::string {aka class std::basic_string<char>}’ through ‘...’
[Details] /home/build/jenkins/workspace/rhel70_atcd_debug_ndds523rev0/BUILD/DOC_ROOT/ACE/ace/Log_Msg.h:110:17: note: in definition of macro ‘ACE_ERROR’
[Details] make[1]: *** [.obj/Annotation_Test.o] Error 1
[

home/build/jenkins/workspace/rhel70_atcd_debug_ndds523rev0/BUILD/DOC_ROOT/TAO/tests/Any/Recursive/GNUmakefile.RecursiveHello_Idl
[Details] *** Error in `/home/build/jenkins/workspace/rhel70_atcd_debug_ndds523rev0/BUILD/DOC_ROOT/ACE/bin/tao_idl': free(): invalid pointer: 0x00000000016042b0 ***
[Details] make[1]: *** [TestC.inl] Aborted
[Details] *** Error in `/home/build/jenkins/workspace/rhel70_atcd_debug_ndds523rev0/BUILD/DOC_ROOT/ACE/bin/tao_idl': free(): invalid pointer: 0x00000000015a62b0 ***
[Details] make[1]: *** [TestS.h] Aborted

jwillemsen · 2020-05-31T13:22:16Z

Looks bcc32 also has some issues https://buildlogs.remedy.nl/win_cbxe13_bcc32_acetao_debug/

MAKE Version 5.43 Copyright (c) 1987, 2019 Embarcadero Technologies, Inc.
bcc32 -v -y -Od -vi- -k -DACE_NO_INLINE=1 -D_DEBUG -tWM -q -tWR -tWD -a8 -DMPC_LIB_MODIFIER=\"_bd\" -DWIN32 -D_WINDOWS -I"X:\BUILD\DOC_ROOT\ACE" -I"." -I"..\..\..\TAO_IDL\include" -I"..\..\..\TAO_IDL\fe" -I"..\..\.." -D_NO_VCL -w-rvl -w-rch -w-ccc -w-obs -w-aus -w-pia -w-inl -w-mls -c -nDebug\annotest_idl\ .\Annotation_Test.cpp
.\Annotation_Test.cpp:
Error E2206 .\Annotation_Test.cpp 519: Illegal character '#' (0x23) in function Annotation_Test::print_idl_with_line_numbers()
Error E2451 .\Annotation_Test.cpp 519: Undefined symbol 'ifndef' in function Annotation_Test::print_idl_with_line_numbers()
Error E2121 .\Annotation_Test.cpp 519: Function call missing ) in function Annotation_Test::print_idl_with_line_numbers()
*** 3 errors in Compile ***

mitza-oci · 2020-05-31T14:46:50Z

@jwillemsen could CI be enhanced to catch these compile-time issues?

@iguessthislldo looks like Annotation_Test.h:64 should be name instead of name_? And Annotation_Test.cpp:519 may need to be refactored to not have a preprocessor conditional within a macro evaluation.

jwillemsen · 2020-05-31T14:49:49Z

Building also all TAO tests will take probably another 2 to 3 hours, that is probably not workable anymore with only 20 workers when there are multiple PRs on a day

mitza-oci · 2020-05-31T17:15:04Z

Building also all TAO tests will take probably another 2 to 3 hours, that is probably not workable anymore with only 20 workers when there are multiple PRs on a day

It would be interesting to try to dynamically determine which tests to build based on the changes from master.

The tao_idl crash in TAO/tests/Any/Recursive

Looks like this is due to using a deleted object valuetype here in line 1147 of the grammar:
scope->fe_add_valuetype (valuetype);

mitza-oci · 2020-05-31T17:23:35Z

The tao_idl crash in TAO/tests/Any/Recursive

Looks like this is due to using a deleted object valuetype here in line 1147 of the grammar:
scope->fe_add_valuetype (valuetype);

This function deletes its first argument and replaces it with a different object when a forward-declared valuetype is being expanded to its fully-declared valuetype (note passing pointer by reference):

              AST_Interface::fwd_redefinition_helper (
                valuetype_as_interface, scope);

valuetype_as_interface is the same object as valuetype, just pointed to a different subobject.

iguessthislldo · 2020-05-31T17:52:41Z

Annotation_Test.cpp:519 may need to be refactored to not have a preprocessor conditional within a macro evaluation.

Using compiler explorer, it looks like gcc and clang accept this, but msvc complains in a similar manner as whatever bcc is. The preprocessor is weird.

I will take a look at all these issues tomorrow.

Fixes for DOCGroup#1125 - TAO_IDL/fe.ypp - Fixed using wrong pointer in grammer. - Other minor tweaks. - tests/IDLv4/annotations - Fixed preprocessor usage in that some compilers don't like. - Fixed passing std::string as a const char*. - Rewrote idl output funciton to be more flexable.

Fixes for DOCGroup#1125 - `TAO_IDL/fe.ypp` - Fixed using wrong pointer in grammar. - Other minor tweaks. - `tests/IDLv4/annotations` - Fixed preprocessor usage that some compilers don't like. - Fixed passing `std::string` as a `const char*`. - Rewrote IDL output function to try to be more flexible.

Fixes for #1125

jwillemsen · 2020-06-03T06:29:53Z

#1132 has fixed the problems in the TAO tests but there looks there is last problem in CIAO, see http://scoreboard.ociweb.com/doc_master_tick_linux_gcc_d0o1b32/index.html

mitza-oci · 2020-06-03T13:47:46Z

#1132 has fixed the problems in the TAO tests but there looks there is last problem in CIAO, see http://scoreboard.ociweb.com/doc_master_tick_linux_gcc_d0o1b32/index.html

Looks like it's an IDL file generated by tao_idl3_to_idl2, in case that matters. I'm also wondering why the Remedy CIAO builds didn't fail?

jwillemsen · 2020-06-03T13:50:05Z

We only compile LwCCM with no CCM events support, looks this test is dependent on CCM events so it is not run in our builds

jwillemsen · 2020-06-03T14:36:03Z

Just made a PR to also build the specific test with noevent, it works on my system, but I don't have the latest tao_idl changes, see DOCGroup/CIAO#126

jwillemsen · 2020-06-03T14:56:15Z

With lwccm+noevent enabled the test runs, so probably it is in the area of event support

jwillemsen · 2020-06-03T15:34:09Z

Callstack, line 6504 did got changed as part of the annotation support

(gdb) back
#0  0x00007ffff7f06d93 in UTL_Scope::fe_add_full_intf_decl<AST_EventType> (this=0x4521e8, t=0x606000) at include/utl_scope_T.cpp:15
#1  0x00007ffff7f2668c in tao_yyparse () at fe/idl.ypp:6504
#2  0x00007ffff7f1f597 in FE_yyparse () at /home/johnny/ACE/trunk/TAO/TAO_IDL/fe/fe_extern.cpp:93
#3  0x000000000040a9d0 in DRV_drive (s=0x451e00 "convert.idl") at /home/johnny/ACE/trunk/TAO/TAO_IDL/tao_idl.cpp:231
#4  0x0000000000405051 in main (argc=<optimized out>, argv=<optimized out>) at /home/johnny/ACE/trunk/TAO/TAO_IDL/tao_idl.cpp:429

iguessthislldo · 2020-06-03T15:44:19Z

Callstack, line 6504 did got changed as part of the annotation support

Okay, that's:

FE_OBVHeader *&event_header = $2;
...
event_header->destroy ();

but this should be the same as what was happening before:

$2->destroy ();

TAO/TAO_IDL/fe/idl.ypp

More Fixes for DOCGroup#1125 Fixed using wrong prointer for add to scope in DOCGroup#1132 for interfaces, now doing that for eventtype and component. See: DOCGroup#1125 (comment)

More Fixes for #1125

iguessthislldo added 3 commits May 28, 2020 20:03

tao_idl: Record lineno of errors and warnings

69e8ba2

Update TAO/NEWS

6d9405d

iguessthislldo requested a review from mitza-oci May 29, 2020 17:00

mitza-oci requested changes May 29, 2020

View reviewed changes

TAO/TAO_IDL/docs/annotations.md Outdated Show resolved Hide resolved

TAO/TAO_IDL/docs/annotations.md Outdated Show resolved Hide resolved

TAO/TAO_IDL/fe/idl.ypp Outdated Show resolved Hide resolved

TAO/TAO_IDL/fe/idl.ypp Show resolved Hide resolved

iguessthislldo added 2 commits May 29, 2020 18:16

TAO IDL FE: Respond to Review in DOCGroup#1125

8a7521d

Update TAO/NEWS

564ef36

mitza-oci reviewed May 29, 2020

View reviewed changes

TAO/TAO_IDL/docs/annotations.md Outdated Show resolved Hide resolved

Tweak TAO_IDL/docs/annotations.md

3021f30

mitza-oci approved these changes May 30, 2020

View reviewed changes

mitza-oci merged commit 57a75e5 into DOCGroup:master May 30, 2020

iguessthislldo mentioned this pull request Jun 2, 2020

Fixes for #1125 #1132

Merged

mitza-oci added a commit that referenced this pull request Jun 2, 2020

Merge pull request #1132 from iguessthislldo/igtd/valuetype_annotations

154c435

Fixes for #1125

iguessthislldo commented Jun 3, 2020

View reviewed changes

TAO/TAO_IDL/fe/idl.ypp Show resolved Hide resolved

iguessthislldo mentioned this pull request Jun 3, 2020

More Fixes for #1125 #1134

Merged

iguessthislldo added a commit that referenced this pull request Jun 4, 2020

Merge pull request #1134 from iguessthislldo/igtd/valuetype_annotations

79af351

More Fixes for #1125

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend Annotation Support #1125

Extend Annotation Support #1125

iguessthislldo commented May 29, 2020

iguessthislldo commented May 29, 2020 •

edited

okellogg commented May 30, 2020

mitza-oci commented May 30, 2020

jwillemsen commented May 31, 2020

jwillemsen commented May 31, 2020

mitza-oci commented May 31, 2020

jwillemsen commented May 31, 2020

mitza-oci commented May 31, 2020

mitza-oci commented May 31, 2020

iguessthislldo commented May 31, 2020

jwillemsen commented Jun 3, 2020

mitza-oci commented Jun 3, 2020

jwillemsen commented Jun 3, 2020

jwillemsen commented Jun 3, 2020

jwillemsen commented Jun 3, 2020

jwillemsen commented Jun 3, 2020

iguessthislldo commented Jun 3, 2020

Extend Annotation Support #1125

Extend Annotation Support #1125

Conversation

iguessthislldo commented May 29, 2020

iguessthislldo commented May 29, 2020 • edited

okellogg commented May 30, 2020

mitza-oci commented May 30, 2020

jwillemsen commented May 31, 2020

jwillemsen commented May 31, 2020

mitza-oci commented May 31, 2020

jwillemsen commented May 31, 2020

mitza-oci commented May 31, 2020

mitza-oci commented May 31, 2020

iguessthislldo commented May 31, 2020

jwillemsen commented Jun 3, 2020

mitza-oci commented Jun 3, 2020

jwillemsen commented Jun 3, 2020

jwillemsen commented Jun 3, 2020

jwillemsen commented Jun 3, 2020

jwillemsen commented Jun 3, 2020

iguessthislldo commented Jun 3, 2020

iguessthislldo commented May 29, 2020 •

edited