Fix generate of @Const on function

[louxiu]

#ifndef CACHE_EXECUTOR_H
#define CACHE_EXECUTOR_H

#include <string>

namespace tutor {

class CacheExecutor {

 public:

  virtual ~CacheExecutor() {}

  virtual void ReadCacheToBuffer(const std::string& key, char buffer, int max_buffer_len) const = 0;
  virtual void WriteCache(const std::string& key, char value) = 0;
  virtual void DeleteCache(const std::string& key) = 0;
};
}
#endif /* CACHE_EXECUTOR_H<INCLUDE>_ */

For the code above, javacpp generates mismatched cpp class.

• Only the first function is const, but in the generated cpp class, all functions is const and @Const is on the class CacheExecutor, not on the function level.
• Missing const for function arguments.

[louxiu]

@saudet PTAL

[saudet]

The @Const on the class is only needed when virtualizing it, and it only works when all virtual functions are const. We'd need to think of something else for when this is not the case. In this case, do you need to subclass it and implement these functions in Java? If not, we don't need to worry about that.

[louxiu]

In this case, do you need to subclass it and implement these functions in Java?

Yes. This is a callback.

If not, we don't need to worry about that.

But the generated cpp class is wrong. Because all the generated function is const. The compiler complains that the real function(eg. WriteCache) is not implemented.

[saudet]

Right, so what about this. Let's use the the third value of @Const to set the const of a member function, in this case @Const({false, false, true}). For backward compatibility though the const of a function should default to true if we have a @Const on the class. Sounds good?

[louxiu]

Oh, I didn't consider backward compatibility problem. Will check it soon.

[louxiu]

Hi @saudet
I have some confusion:

• @Const has two value. When it is putted on the parameter, the first means const? and the second means const * ?
• When @Const is on the class, what do the two values mean?
• Can @Const be putted on the function (I thought the answer is yes)? It means the function is const or it is related to the return value?

[louxiu]

I can't understand why solving the compatibility problem needs to add the third value.

As I understand, the compatibility problem is there is code that @Const is on the class. so we still need to handle it(It means the class is virtual and all the function is const). So the function is const when @Const on the class or @Const on the function.

[saudet]

Yes, the first one is for a value like const char* and the second for a pointer like char const *. The second value isn't used when annotated to a FunctionPointer (or a virtual class in which case it's applied to all virtual functions). There's only one meaning for const on methods in C++ so no need for the second value. At the moment, @Const on the method is applied to the return value/pointer. So to get something that applies to the method, it makes sense to use the third value of @Const, right? They appear in that order in a C++ declaration like const char const * f() const.

Right now putting @Const on the class sets all the virtual methods to const, so we need a way to disable that with @Const({?, ?, false}) as well. The latter should override the former.

[louxiu]

Got it 😄 . @saudet Please take a look.

[saudet]

Looks better, thanks! But I can see at least 2 remaining problems:

1. We can't just append @Const({false, false, true}) if there is already a @Const for the return value. We'll have to at least yank it out, and ideally incorporate its values, but let's worry about that later.
2. We should leave the default value of the annotation to {true, false}, that is with a missing third value, in which case it defaults to whether @Const appears on the class, for backward compatibility.

[louxiu]

You are right. Patch is added.

[saudet]

Ok, looking good! I fixed 2 remaining small issues in the commit above. Let me know if this is alright and we'll be able to merge this. Thanks!

[louxiu]

Ok, thank you for your patience. 😄

[saudet]

Thanks to you for your contribution!