IDLv4 Explicitly-named Integer Types #840

iguessthislldo · 2019-02-21T03:51:24Z

Implement part of IDL4.2 Section 7.4.13 "Building Block Extended Data-Types" by implementing:

7.4.13.4.4 "Integers restricted to holding 8-bits of information": int8 and uint8
- New integer types that have to be distinguished from the existing octet and char types.
7.4.13.4.5 "Explicitly-named Integer Types": int16, uint16, int32, uint32, int64, and uint64
- Just aliases of the existing integer types

uint16, uint32, uint64, int16, int32, int64 from IDL 4.2 section 7.4.13.4.5 "Explicitly-named Integer Types" WIP uint8/int8 from IDL4.2 section 7.4.13.4.4 "Integers restricted to holding 8-bits of information"

jwillemsen · 2020-06-30T11:14:46Z

Any updates on this PR, do we need to keep this open?

iguessthislldo · 2020-07-02T05:52:48Z

Any updates on this PR, do we need to keep this open?

I don't see a reason to close it as I plan to come back to it someday and it also keeps the work visible to someone who might be interested. If it's an issue of time there are older PRs.

And a bunch of fixes to get it to (sorta) work.

- Add uint8 and int8 to ACE and TAO serialization - Refactor parts of AST_Expression to also cover uint8 and int8

jwillemsen · 2021-06-13T08:29:05Z

New test should be in TAO/bin/tao_orb_tests.lst, msvc doesn't compile and has some new warnings, will review when the checks are all ok

iguessthislldo · 2021-06-13T21:03:32Z

msvc doesn't compile and has some new warnings,

I can fix the warnings in AST_Expression.cpp, but for the error this says it's an I/O error:

    263>CustomBuild:
         D:\a\ACE_TAO\ACE_TAO/ACE\bin\tao_idl: Unable to rename temporary file "C:\Users\RUNNER~1\AppData\Local\Temp\tao-idlf_N2p6U3" to "C:\Users\RUNNER~1\AppData\Local\Temp\tao-idlf_N2p6U3.cpp": Input/output error
   263>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(241,5): error MSB8066: Custom build for 'AnySeq.pidl;Bounds.pidl;Dynamic.pidl;Dynamic_Parameter.pidl;ValueModifier.pidl;Visibility.pidl' exited with code 1. [D:\a\ACE_TAO\ACE_TAO\TAO\tao\AnyTypeCode\AnyTypeCode_Idl.vcxproj]
   263>Done Building Project "D:\a\ACE_TAO\ACE_TAO\TAO\tao\AnyTypeCode\AnyTypeCode_Idl.vcxproj" (default targets) -- FAILED.

I don't know the cause of this particular error, but if there is chance for the temporary file to problematic on Windows, it would be fixed by avoiding using a temporary file in #1357.

ACE/ace/CDR_Base.h

TAO/TAO_IDL/be/be_predefined_type.cpp

TAO/tests/IDLv4/explicit_ints/main.cpp

TAO/TAO_IDL/fe/fe_lookup.cpp

TAO/TAO_IDL/ast/ast_expression.cpp

TAO/TAO_IDL/include/ast_expression.h

TAO/tao/CDR.h

DOCGroup#840

TAO/TAO_IDL/be/be_helper.cpp

TAO/TAO_IDL/include/ast_expression.h

TAO/TAO_IDL/be/be_helper.cpp

TAO/TAO_IDL/ast/ast_expression.cpp

DOCGroup#840

ACE/ace/CDR_Base.h

jwillemsen · 2021-06-16T06:34:20Z

ACE/ace/Basic_Types.h

-#define ACE_CHAR_MAX 0x7F
-#define ACE_CHAR_MIN -(ACE_CHAR_MAX)-1
-#define ACE_OCTET_MAX 0xFF
+#define ACE_INT8_MAX 0x7F


We should convert these to constexpr at some point

We should, but I'd like that to be later because I don't want to make this harder to backport than it already is.

TAO/TAO_IDL/ast/ast_expression.cpp

jwillemsen · 2021-06-16T06:47:32Z

TAO/tao/Basic_Types_IDLv4.h

@@ -0,0 +1,58 @@
+#ifndef TAO_BASIC_TYPES_IDLV4_H


Why not put these in Basic_Types.h and in the CORBA namespace, I think we should support CORBA::Int8 without the requirement that the user includes this file, makes it much easier to use this construct which is also much clear in application code. For TAOX11 we could use this also without a special include and a namespace using.

int8 is not a CORBA type (it has no Typecode, Any, IfR, DII, DSI, shouldn't be used for remote calls...).
For this whole group of type names, we wanted to make it explicit that these are different - tao_idl with default arguments will not accept them, they're not "peers" of the other names.

Ok, we could keep Int8/UInt8 in this file, but the others are just typedefs which could be used without problems. That would enable TAOX11 to easily use these internally without a special include

jwillemsen

We shouldn't introduce a new CORBA::IDLv4 namespace

jwillemsen · 2021-06-22T06:28:15Z

ACE/ace/CDR_Stream.h

@@ -791,6 +807,18 @@ class ACE_Export ACE_InputCDR
    ACE_CDR::Octet &ref_;
  };

+  struct ACE_Export to_int8


Is this really required, isn't int8_t a distinct type itself?

(u)int*_t are typedefs, probably at least one is the same as the one used for ACE_CDR's Octet, etc.

jwillemsen · 2021-06-22T06:32:16Z

TAO/tao/tao.mpc

@@ -60,6 +60,12 @@ project(TAO_Core_idl) : tao_versioning_idl_defaults, gen_ostream, install, pidl_
    WStringSeq.pidl >> AnyTypeCode/WStringSeqA.h AnyTypeCode/WStringSeqA.cpp
  }

+  IDL_Files {
+    idlflags += -Sci -Gse -Gata --idl-version 4
+    Int8Seq.pidl >> AnyTypeCode/Int8SeqA.h AnyTypeCode/Int8SeqA.cpp


New Int8SeqA/UInt8SeqA needs to be added to AnyTypeCode

That's not in scope. Can I just remove those parts of the line?

Are the files generates but empty? Then just add them, we shouldn’t have generated files which we don’t compile

mitza-oci

Still needs some CI fixes and to resolve @jwillemsen's comment about AnyTypeCode. But the review itself is good to go.

jwillemsen · 2021-07-01T06:09:20Z

@iguessthislldo Several new warnings in the https://buildlogs.remedy.nl/win_vs2019_tao_debug/index.html build, can you have a look?

jwillemsen · 2021-07-01T16:25:42Z

Can you also check the Coverity scan results, we have fresh results, several new issues related to this PR

iguessthislldo · 2021-07-01T18:32:04Z

Yes, I will look into both, but for the warnings I don't think I will be able to tell if there are any warnings I need to fix outside the explicit_ints test.

DOCGroup#840 Because the main set of issues in `coerce_value` was caused by applying the existing pattern, the issues caused by the existing code should also be fixed.

Ported from DOCGroup#840 to ACE6/TAO2. Also regenerated with Bison 3.7.6 at the same time like in d581918 in DOCGroup#1594

DOCGroup#840 Because the main set of issues in `coerce_value` was caused by applying the existing pattern, the issues caused by the existing code should also be fixed.

Backport of DOCGroup#1715 Fixes DOCGroup#1713 and adds some regression tests for constant expressions. Also tried to do some more cleanup and minor fixes related to DOCGroup#840.

iguessthislldo added 6 commits January 21, 2019 14:19

TAO_IDL: IDL4 stdint-like Integers

8f4fcb1

uint16, uint32, uint64, int16, int32, int64 from IDL 4.2 section 7.4.13.4.5 "Explicitly-named Integer Types" WIP uint8/int8 from IDL4.2 section 7.4.13.4.4 "Integers restricted to holding 8-bits of information"

Add new limits for new types to Basic_Types.h

c7a6292

ast_constant.cpp: Move C++ specific code out of the FE

ce28b30

Adding Files As is For Merge

d004d8d

Merge remote-tracking branch 'upstream/master' into igtd/stdint

9d2523a

tao_idl: Committing Parser as is

7549f0b

jwillemsen mentioned this pull request Mar 12, 2021

error in lookup of symbol uint32? OpenDDS/OpenDDS#2483

Closed

iguessthislldo added 3 commits June 7, 2021 17:12

Merge remote-tracking branch 'upstream/master' into igtd/stdint

85a1e61

Added WIP explicit_ints Test

a4c5b22

And a bunch of fixes to get it to (sorta) work.

Fix Issues with explicit_ints Test

56a61b9

- Add uint8 and int8 to ACE and TAO serialization - Refactor parts of AST_Expression to also cover uint8 and int8

iguessthislldo marked this pull request as ready for review June 13, 2021 03:49

iguessthislldo changed the title ~~stdint-like integer types in IDLv4~~ IDLv4 Explicitly-named Integer Types Jun 13, 2021

iguessthislldo requested review from jwillemsen and mitza-oci June 13, 2021 03:51

CDR_Stream.h: Fix Arg Type

e994776

Fix Warnings, Add explicit_ints to Test List

2afc6a2

jwillemsen reviewed Jun 14, 2021

View reviewed changes

ACE/ace/CDR_Base.h Outdated Show resolved Hide resolved

jwillemsen reviewed Jun 14, 2021

View reviewed changes

TAO/TAO_IDL/be/be_predefined_type.cpp Outdated Show resolved Hide resolved

jwillemsen reviewed Jun 14, 2021

View reviewed changes

TAO/tests/IDLv4/explicit_ints/main.cpp Outdated Show resolved Hide resolved

mitza-oci requested changes Jun 14, 2021

View reviewed changes

TAO/TAO_IDL/fe/fe_lookup.cpp Show resolved Hide resolved

TAO/TAO_IDL/ast/ast_expression.cpp Outdated Show resolved Hide resolved

TAO/TAO_IDL/include/ast_expression.h Show resolved Hide resolved

TAO/tao/CDR.h Outdated Show resolved Hide resolved

Partially Respond to Review of DOCGroup#840

737b919

DOCGroup#840

mitza-oci requested changes Jun 14, 2021

View reviewed changes

iguessthislldo added 2 commits June 14, 2021 18:28

Respond to Review of DOCGroup#840

2280b39

DOCGroup#840

ast_expression.cpp: Add Include for sprintf

7471fc9

jwillemsen reviewed Jun 15, 2021

View reviewed changes

ACE/ace/CDR_Base.h Outdated Show resolved Hide resolved

s/Uint/UInt/g

800cec4

jwillemsen reviewed Jun 16, 2021

View reviewed changes

TAO/TAO_IDL/ast/ast_expression.cpp Outdated Show resolved Hide resolved

jwillemsen reviewed Jun 16, 2021

View reviewed changes

jwillemsen requested changes Jun 16, 2021

View reviewed changes

Support int8/uint8 Sequences and Arrays

2ed7fe7

iguessthislldo requested review from jwillemsen and mitza-oci June 22, 2021 01:25

jwillemsen reviewed Jun 22, 2021

View reviewed changes

mitza-oci approved these changes Jun 23, 2021

View reviewed changes

iguessthislldo added 2 commits June 29, 2021 18:53

Try to Fix Issues with int8/uint8

70b28ba

Fix Issue with No Inlining

354b698

iguessthislldo force-pushed the igtd/stdint branch from f32e89c to 354b698 Compare June 30, 2021 03:40

iguessthislldo requested review from jwillemsen and mitza-oci June 30, 2021 05:36

jwillemsen approved these changes Jun 30, 2021

View reviewed changes

iguessthislldo merged commit 48f51b5 into DOCGroup:master Jun 30, 2021

iguessthislldo added a commit to iguessthislldo/ACE_TAO that referenced this pull request Jul 9, 2021

IDLv4 Explicitly-named Integer Types

2cbdf73

Ported from DOCGroup#840 to ACE6/TAO2. Also regenerated with Bison 3.7.6 at the same time like in d581918 in DOCGroup#1594

This was referenced Jul 9, 2021

[ACE6] IDLv4 Explicitly-named Integer Types #1595

Merged

Fix Coverity Issues and Compiler Warnings #1594

Merged

This was referenced Nov 3, 2021

tao_idl fails to compile const double variable assignments with math #1713

Closed

Fix Floating Point Constant Expressions in tao_idl #1715

Merged

iguessthislldo mentioned this pull request Nov 9, 2021

[TAO 2] Fix Floating Point Constant Expressions in tao_idl #1717

Merged

iguessthislldo mentioned this pull request Sep 2, 2022

IDL4 keyword escaping needs to be extended with the new keywords #1256

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IDLv4 Explicitly-named Integer Types #840

IDLv4 Explicitly-named Integer Types #840

iguessthislldo commented Feb 21, 2019 •

edited

Loading

jwillemsen commented Jun 30, 2020

iguessthislldo commented Jul 2, 2020

jwillemsen commented Jun 13, 2021

iguessthislldo commented Jun 13, 2021

jwillemsen Jun 16, 2021

iguessthislldo Jun 18, 2021

jwillemsen Jun 16, 2021

mitza-oci Jun 16, 2021

jwillemsen Jun 16, 2021

jwillemsen left a comment

jwillemsen Jun 22, 2021

mitza-oci Jun 22, 2021

jwillemsen Jun 22, 2021

iguessthislldo Jun 22, 2021

jwillemsen Jun 22, 2021

mitza-oci left a comment

jwillemsen commented Jul 1, 2021

jwillemsen commented Jul 1, 2021

iguessthislldo commented Jul 1, 2021

IDLv4 Explicitly-named Integer Types #840

IDLv4 Explicitly-named Integer Types #840

Conversation

iguessthislldo commented Feb 21, 2019 • edited Loading

jwillemsen commented Jun 30, 2020

iguessthislldo commented Jul 2, 2020

jwillemsen commented Jun 13, 2021

iguessthislldo commented Jun 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwillemsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitza-oci left a comment

Choose a reason for hiding this comment

jwillemsen commented Jul 1, 2021

jwillemsen commented Jul 1, 2021

iguessthislldo commented Jul 1, 2021

iguessthislldo commented Feb 21, 2019 •

edited

Loading