JSONCompilationDB Parser

Why is it so strict? I am trying to write an ide backend would like to use the compilation commands file to contain some extra information so that i don’t have to keep multiple files. the following code in parse message doesn’t allow me to keep any other key but directory, command and file.

else {
ErrorMessage = (“Unknown key: “” +
KeyString->getRawValue() + “””).str();
return false;
}

Could this be removed?

Hi Ramneek,

Hi Manuel,

I see your point. However, being strict has one large problem. If we at some point decide to add additional information to the compilation database, all previously released libclang tools will break. This means, this restriction makes it even for ourselves unnecessarily hard and disruptive to extend the compilation db format.
If possible, I would be very much in favor of removing this restriction even before clang 3.2.

Cheers,
Tobi

If multiple people vote for it, I can most certainly be convinced
otherwise...

Cheers,
/Manuel

Manuel Klimek wrote:

If we really want to do this to deliberately allow 3rd party fields in
the database it's probably worthwhile having some kind of defined
namespace (either for our names or for the 3rd party names) so that
future extensions to our schema don't conflict with 3rd party fields.

(& in that case we could still hard-error (if we want to) on fields in
our namespace that are unknown (Tobias's position to be considered -
only works if new fields are always acceptable to be ignored))

If we really want to do this to deliberately allow 3rd party fields in
the database it's probably worthwhile having some kind of defined
namespace (either for our names or for the 3rd party names) so that
future extensions to our schema don't conflict with 3rd party fields.

Having namespaces for third-party plugins is a good idea.

(& in that case we could still hard-error (if we want to) on fields in
our namespace that are unknown (Tobias's position to be considered -
only works if new fields are always acceptable to be ignored))

Regarding our (or the global) namespace, it would be interesting to know if we expect additions that are not acceptable to be ignored. I assumed additional fields provide additional information that would just be missing if the fields are not read. Similar to libclang.so we could provide additional information in new fields, but we could agree to not change the semantics of existing fields.

I am mainly concerned about backwards compatibility issues. It may be helpful to clarify if we want to remain compatible with older versions of clang and in case we do, how we want to ensure compatibility. Is my understanding correct that the current code will make clang versions error out, in case they read a compilation database that includes extensions we may possibly add ourselves at some point in the future? In case this is true, how would we handle such a situation? Are there other solutions than requiring the user to upgrade to the most recent version of clang (possibly not yet in his package repository or not installed by his sysadmins)?

Cheers
Tobi

Would it be ok if i worked on a patch to move things to a namespace for the first cut while we decide how we can allow plugins.
Also we should be ignoring the namespaces that we are not using so it allows applications to add their custom data to while preserving the acceptable file format.

My proposal is actually simple:

This is what we have today:

[
  { "directory": "/home/user/llvm/build",
    "command": "/usr/bin/clang++ -Irelative -DSOMEDEF='\"With spaces and quotes.\"' -c -o file.o file.cc",
    "file": "file.cc" },
  …
]
We can move it to (cc = compile commands):
{ "cc" : { "directory": "/home/user/llvm/build",
    "command": "/usr/bin/clang++ -Irelative -DSOMEDEF='\"With spaces and quotes.\"' -c -o file.o file.cc",
    "file": "file.cc" },
  …
]

One thing that we will have to be careful about is that we will have to patch the cmake compilation command generation facility as well..
I can look into that as well or make the change such that it is backwards compatible?

Regards

If we really want to do this to deliberately allow 3rd party fields in
the database it's probably worthwhile having some kind of defined
namespace (either for our names or for the 3rd party names) so that
future extensions to our schema don't conflict with 3rd party fields.

Having namespaces for third-party plugins is a good idea.

I also like the idea of a reserved namespace for 3rd party extensions.

Cheers,
Arnaud

Would it be ok if i worked on a patch to move things to a namespace for
the first cut while we decide how we can allow plugins.
Also we should be ignoring the namespaces that we are not using so it
allows applications to add their custom data to while preserving the
acceptable file format.

My proposal is actually simple:

This is what we have today:

[
  { "directory": "/home/user/llvm/build",
    "command": "/usr/bin/clang++ -Irelative -DSOMEDEF='\"With spaces and quotes.\"' -c -o file.o file.cc",
    "file": "file.cc" },
  …
]

We can move it to (cc = compile commands):

{ "cc" : { "directory": "/home/user/llvm/build",

    "command": "/usr/bin/clang++ -Irelative -DSOMEDEF='\"With spaces and quotes.\"' -c -o file.o file.cc",
    "file": "file.cc" },
  …
]

One thing that we will have to be careful about is that we will have to patch the cmake compilation command generation facility as well..

I can look into that as well or make the change such that it is backwards compatible?

I'm still torn. While I see the arguments for why being strict might be a
problem if we plan to change the format, I don't understand the need for
the namespace / being able to have different things in a single file yet.

Cheers,
/Manuel

I also have no opinion about name spaces. However, I think it would be
good if we could ensure clang 3.2 just ignores unknown content. Otherwise, the costs of adding new information to that file will be a lot higher.

Manuel, do you think this change would still be OK for 3.2?

Cheers
Tobias

Folks, 3.2 is essentially shipped. This should *not* hold up that release.

I don't think that the clang tools are sufficiently mature to really spend
a lot of time or effort trying to make the 3.2 release versions of the
tools work with future build systems and/or IDEs. If we wanted this to be
true, we should have done a lot more testing of these tools prior to the
release.

I think we should make including a collection of clang-based tools a target
for the 3.3 release, and try to actually advertise the fact that we're
going to try to do this, test them thoroughly, and of course both make and
document these types of decisions. But not 3.2, it's too late in the
release game to start thinking about this IMO.

Note that I am not (yet) taking a stance about the actual issue this patch
is trying to address here, just about the release implications or lack
thereof.

Agreed. This is really not a release blocker.

On the other hand, as this is a very non-critical component, getting this small change in may still be possible, and it may make our life a little simpler in the future. To my knowledge, there is another rc3 scheduled for Wednesday, so that could be the last window to get trivial changes in.

Manuel, as you are the tooling stuff expert, I leave the decision to you. I did my job of raising the topic. :wink:

All the best
Tobi

I agree with Chandler that trying to get this into 3.2 is not going to be
worth the effort. We should rather discuss the merits of the approaches on
a principled basis and make that decision going forward.

Cheers,
/Manuel

Thanks guys. When is the next release and where can i look at the release calendar?

-RH

Manuel Klimek wrote:

I agree with Chandler that trying to get this into 3.2 is not going to be
worth the effort. We should rather discuss the merits of the approaches on
a principled basis and make that decision going forward.

Yes.

I still don't see how previously released clang tools would break if the
format was extended.

http://thread.gmane.org/gmane.comp.compilers.clang.devel/25882/focus=25890

Can someone say how, and say what assumptions about source compatibility
would have to be made in order to break a tool?

Thanks,

Steve.