WIP: Removed BasicGrammar_inl.h and all macros inside it #616
Conversation
|
Before we merge, please also verify on the testing mac that it works. |
|
I first need to make it work on linux actually ! But yes I'll check on Mac when I'm there. |
gaede
changed the title
|
Before this request is merged I suggest to discuss the implications in one of the following dd4hep developer meetings. |
sponce
force-pushed
the
sponce_grammacroremoval
branch
from
85d78be
to
de74998
Compare
|
Second attempt at this. This time all tests pass, but the change is not fully backward compatible (see end of the release notes and last commit). I could make it backward compatible by creating all grammars in libDDCore just in case, as it was before basically, but I find this really an overkill when you need only one of of 50 and when 99% of the time you still do not have anything to do. |
sponce
force-pushed
the
sponce_grammacroremoval
branch
from
de74998
to
a99dc32
Compare
|
Not yet fine... it fails again on Mac... Good that I've checked. I'm on it |
sponce
force-pushed
the
sponce_grammacroremoval
branch
from
a99dc32
to
86b689f
Compare
…ammar_inl.h So far : - create a commonGramar base class for the Grammar class in order to be able to specialize easily the templated Grammar class - specialized the Grammar class for std::vector - dropped the macros concerning vector grammars, as they are replaced by the specialization Note that the specialization is much shorter in code and much more generic than the macros, as the compiler will generate appropriate Grammars for new type without any change of code
…the templated Grammar
sponce
force-pushed
the
sponce_grammacroremoval
branch
3 times, most recently
from
bbf16d4
to
9c080ca
Compare
|
ok, now also working on Mac :-) |
|
you still have problems with xerces https://gitlab.cern.ch/mpetric/DD4hep/pipelines/1414233 |
As explained in the release notes, no "default" grammars are provided anymore, thus the user may need to instantiate them explicitly in some cases. The commit "Reintroduce missing dictionaries in examples" is about that for the examples we have. As you can see, out of >200 examples, only 3 files were concerned. These are those where we "load" conditions from an external source, e.g. ROOT file, without knowing a priori what will come out. In such cases, the current code already expects the needed dictionary to be "magically" present : it does not call In case of external types, this is done by calling some macros and it's still unchanged. But for "standard" types (to be defined actually), dictionaries were provided directly, I believe in libDDCore.so. So for double, vector and many others, you did not have to do anything. This had 2 major drawbacks in my opinion : first that it's a lottery, e.g. you have So now even "standard" types' grammars have to be explicitly instantiated, like in the mentioned commit. However, in most cases, the explicit usage of a condition of these types triggers the instantiation of the Grammar via the templating machinery. It's only when the type is never mentioned that we need an extra line of code. I hope this was clearer ! |
sponce
force-pushed
the
sponce_grammacroremoval
branch
from
d04cc01
to
b361670
Compare
sponce
force-pushed
the
sponce_grammacroremoval
branch
from
b361670
to
bc40d56
Compare
|
Problems with Xerces should now be fixed. It's sad that this is not checked by the CI. Thanks for spotting them @petricm |
|
Thank you for fixing, but on Mac with Xerces it still does not work |
|
Should be fixed now. |
|
Ok. Can you point me to the changes of behavior so that I have them in mind ? |
|
Ah and we should also add a test that checks these changes, so that I cannot break things again. |
|
It's the CMS change described #628 |
BEGINRELEASENOTES
config.h, eval_none function and parse_none functionsENDRELEASENOTES